RESOLVED FIXED 8455
selectedIndex for a select after a Form reset() has wrong value 
https://bugs.webkit.org/show_bug.cgi?id=8455
Summary selectedIndex for a select after a Form reset() has wrong value
Christos Zisopoulos
Reported 2006-04-18 06:44:37 PDT
In a simple for with a <select/> two <option/> tags, when a page loads selectedIndex is 0 (the first option). After calling document.forms[0].reset(), selectedIndex is -1 Only by explicitly specifying an option as selected, will you get the expected behaviour after a reset. <html> <head> <title>test</title> <script type="text/javascript" language="javascript" charset="utf-8"> function selectBug() { alert("Before Form.reset() selectedIndex is " + document.forms[0].elements[0].selectedIndex); document.forms[0].reset(); alert("AFTER Form.reset() selectedIndex is " + document.forms[0].elements[0].selectedIndex); } </script> </head> <body onload="selectBug()"> <form action="test_submit" method="post"> <select id="select_1" name="event[event_type_id]"> <option value="1" >Value 1</option> <option value="2" selected>Value 2</option> </select> <input name="commit" type="submit" value="Submit" /> </form> </body> </html>
Attachments
a simple demonstration of the bug (1.16 KB, text/html)
2006-04-18 06:49 PDT, Christos Zisopoulos 		no flags
Details
Fix bug in testcase (1.16 KB, text/html)
2006-05-23 12:30 PDT, Rob Buis 		no flags
Details
First attempt (1.56 KB, patch)
2006-05-23 12:48 PDT, Rob Buis 		ggaren: review-
DetailsFormatted DiffDiff
Light version (3.57 KB, patch)
2006-05-24 02:05 PDT, Rob Buis 		ggaren: review+
DetailsFormatted DiffDiff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.
Christos Zisopoulos
Comment 1 2006-04-18 06:49:33 PDT
Created attachment 7800 [details] a simple demonstration of the bug
Christos Zisopoulos
Comment 2 2006-04-18 06:50:52 PDT
Possible duplicate of #6282?
Alexey Proskuryakov
Comment 3 2006-04-18 13:12:23 PDT
Confirmed with stock 10.4.5 Safari. ToT fails with an assertion failure, thus upgrading to P2. ASSERTION FAILED: firstOption == listItems.size() || found (/Users/ap/WebKit/WebCore/rendering/render_form.cpp:904 void WebCore::RenderSelect::updateSelection())
Rob Buis
Comment 4 2006-05-23 12:30:14 PDT
Created attachment 8490 [details] Fix bug in testcase I am pretty sure the testcase wanted to use forms[1] reset, otherwise testing for selectedIndex of forms[1] is pointless. Cheers, Rob.
Rob Buis
Comment 5 2006-05-23 12:48:11 PDT
Created attachment 8491 [details] First attempt First stab at fixing this. It may be too "heavy", in which case I'd like to hear how to make it "lighter" :) Ofcourse if the code is okayed, I'll provide a testcase... Cheers, Rob.
Geoffrey Garen
Comment 6 2006-05-23 13:38:35 PDT
Comment on attachment 8491 [details] First attempt It should be sufficient to keep a flag tracking whether any of the option elements had the 'selected' attribute set, and select option 0 if none did. That would avoid the call to recalcListItems(). I'm going to mark this r-, in case you want to submit the 'lighter' solution.
Rob Buis
Comment 7 2006-05-24 02:05:19 PDT
Created attachment 8510 [details] Light version Hi Geoff, Here is my "light" version including testcase. Cheers, Rob.
Geoffrey Garen
Comment 8 2006-05-24 07:26:06 PDT
Comment on attachment 8510 [details] Light version firstOption is unnecessary. You can just use items[0]. That's a nitpick, though. Nice work.
Rob Buis
Comment 9 2006-05-24 14:16:21 PDT
Hi, (In reply to comment #8) > (From update of attachment 8510 [details] [edit]) > firstOption is unnecessary. You can just use items[0]. That's a nitpick, > though. Nice work. > As discussed on irc, this is mostly a preference thing. If always item[0] was the correct one, I'd agree, but it doesnt have to be, so we'd need a while loop. I do not think either solution is perfect, though I slightly prefer the current one. Cheers, Rob.
Note You need to log in before you can comment on or make changes to this bug.