Bug 60986

Summary: REGRESSION (r80808): Multiple <select> - Selection reset to first element from multiple selected ones
Product: WebKit Reporter: Cristian Vat <cristian.vat>
Component: FormsAssignee: Emil A Eklund <eae>
Status: RESOLVED FIXED    
Severity: Normal CC: ademar, ap, darin, dglazkov, eae
Priority: P1 Keywords: InRadar, Regression
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Attachments:
Description Flags
reduced test case
none
Patch
none
Archive of layout-test-results from cr-jail-8
none
Patch for landing none

Cristian Vat
Reported 2011-05-17 14:16:32 PDT
Created attachment 93816 [details] reduced test case HTML/DOM: A <select> with multiple active and two or more <option>s already in selected state. Action: selectElement.multiple=true Behavior: Only the first <option> which is selected is kept as selected. The rest are displayed as not selected. Expected: already selected elements should not be/appear deselected Reduced test case attached. Tested on nightly WebKit-r86672, Windows 7 x64 Issue originally reported in chromium at http://code.google.com/p/chromium/issues/detail?id=80672
Attachments
reduced test case (451 bytes, text/html)
2011-05-17 14:16 PDT, Cristian Vat 		no flags
Details
Patch (4.61 KB, patch)
2011-05-18 14:05 PDT, Emil A Eklund 		no flags
DetailsFormatted DiffDiff
Archive of layout-test-results from cr-jail-8 (208.42 KB, application/zip)
2011-05-18 17:40 PDT, WebKit Commit Bot 		no flags
Details
Patch for landing (4.92 KB, patch)
2011-05-18 18:28 PDT, Emil A Eklund 		no flags
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Alexey Proskuryakov
Comment 1 2011-05-17 22:40:10 PDT
Regressed between r80761 and r80833, meaning that it's almost certainly <http://trac.webkit.org/r80808>.
Emil A Eklund
Comment 2 2011-05-18 10:42:55 PDT
Thanks Alexey, I'll look into it.
Emil A Eklund
Comment 3 2011-05-18 14:05:29 PDT
Created attachment 93981 [details] Patch
Alexey Proskuryakov
Comment 4 2011-05-18 14:58:53 PDT
<rdar://problem/9464161>
WebKit Commit Bot
Comment 5 2011-05-18 17:40:40 PDT
Comment on attachment 93981 [details] Patch Rejecting attachment 93981 [details] from commit-queue. Failed to run "['./Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '--bot-id=cr-jail-8', 'build-..." exit_code: 2 Last 500 characters of output: tests/xmlhttprequest ................................................................................................................................................................................ http/tests/xmlhttprequest/web-apps ............... http/tests/xmlhttprequest/workers ........... http/tests/xmlviewer . http/tests/xmlviewer/dumpAsText ........... 739.17s total testing time 23575 test cases (99%) succeeded 1 test case (<1%) had incorrect layout 14 test cases (<1%) had stderr output Full output: http://queues.webkit.org/results/8710665
WebKit Commit Bot
Comment 6 2011-05-18 17:40:44 PDT
Created attachment 94011 [details] Archive of layout-test-results from cr-jail-8 The attached test failures were seen while running run-webkit-tests on the commit-queue. Bot: cr-jail-8 Port: Mac Platform: Mac OS X 10.6.7
Emil A Eklund
Comment 7 2011-05-18 18:28:26 PDT
Created attachment 94022 [details] Patch for landing
WebKit Commit Bot
Comment 8 2011-05-19 02:22:09 PDT
Comment on attachment 94022 [details] Patch for landing Clearing flags on attachment: 94022 Committed r86827: <http://trac.webkit.org/changeset/86827>
Cristian Vat
Comment 9 2011-05-19 07:29:53 PDT
Just tested with nightly WebKit-r86842 and it now works correctly for me.
Emil A Eklund
Comment 10 2011-05-19 09:46:58 PDT
Thanks for verifying.
Darin Adler
Comment 11 2011-05-19 10:15:15 PDT
It doesn’t make sense that setting multiple with the binding directly does something different than setting the multiple attribute; all the code to handle the change from single to multiple needs to be in the attribute handling/parsing code and none should be in the setMultiple function. I think all the code in setMultiple is probably wrong. We should test the behavior in other browsers to be sure but I think that is the way to go and this refinement of the code is not sufficient.
Emil A Eklund
Comment 12 2011-05-19 10:32:14 PDT
Alright, I'll see if I can move this logic to the attribute handling code. I'm fairly certain that the current behavior matches that of IE and Firefox but I'll verify that.
Ademar Reis
Comment 13 2011-05-19 13:39:25 PDT
Revision r86827 cherry-picked into qtwebkit-2.2 with commit f46a068 <http://gitorious.org/webkit/qtwebkit/commit/f46a068>
Emil A Eklund
Comment 14 2011-05-24 17:46:08 PDT
Filed bug 61406 to track cleanup suggested by Darin.
Note You need to log in before you can comment on or make changes to this bug.