REGRESSION (r80808): Multiple <select> - Selection reset to first element from multiple selected ones

• Component: Forms
• Status: RESOLVED FIXED

[Cristian Vat]

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

[Alexey Proskuryakov]

Regressed between r80761 and r80833, meaning that it's almost certainly <http://trac.webkit.org/r80808>.

[Emil A Eklund]

Thanks Alexey, I'll look into it.

[Emil A Eklund]

Created attachment 93981 [details]
Patch

[Alexey Proskuryakov]

<rdar://problem/9464161>

[WebKit Commit Bot]

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]

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]

Created attachment 94022 [details]
Patch for landing

[WebKit Commit Bot]

Comment on attachment 94022 [details]
Patch for landing

Clearing flags on attachment: 94022

Committed r86827: <http://trac.webkit.org/changeset/86827>

[Cristian Vat]

Just tested with nightly WebKit-r86842 and it now works correctly for me.

[Emil A Eklund]

Thanks for verifying.

[Darin Adler]

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]

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]

Revision r86827 cherry-picked into qtwebkit-2.2 with commit f46a068 <http://gitorious.org/webkit/qtwebkit/commit/f46a068>

[Emil A Eklund]

Filed bug 61406 to track cleanup suggested by Darin.