Summary: | Setting value on a select element to a non existing option value should clear selection | ||||||
---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Erik Arvidsson <arv> | ||||
Component: | Forms | Assignee: | Jon Lee <jonlee> | ||||
Status: | RESOLVED FIXED | ||||||
Severity: | Normal | CC: | ap, ian, jonlee, rafaelw, webkit-bug-importer, webkit.review.bot | ||||
Priority: | P2 | Keywords: | InRadar | ||||
Version: | 528+ (Nightly build) | ||||||
Hardware: | All | ||||||
OS: | All | ||||||
URL: | http://www.whatwg.org/specs/web-apps/current-work/#dom-select-value | ||||||
Attachments: |
|
Description
Erik Arvidsson
2011-08-30 14:37:28 PDT
This fails in WebKit and in Firefox, so sounds like a mistake in the spec. IE9 follows the spec. FWIW (probably not much), the spec seems like the right (desirable) behavior to me. WebKit/Gecko behavior doesn't seem unnatural or confusing to me. Why base the spec on a minority engine behavior? Is there known Web compatibility impact? Created attachment 120984 [details]
Patch
Attachment 120984 [details] did not pass style-queue:
Failed to run "['Tools/Scripts/update-webkit']" exit_code: 9
Updating OpenSource
Index mismatch: 3a7674dadd24ecd6a1b023eba203ecf2ed62fc59 != edb861612940a3244431d239dacdbc36317b627c
rereading e1e2538cff7d320c2b7cc22c0b75c9369ce9cdb2
A LayoutTests/svg/custom/webkit-transform-crash.html
A LayoutTests/svg/custom/webkit-transform-crash-expected.txt
M LayoutTests/ChangeLog
M Source/WebCore/ChangeLog
M Source/WebCore/svg/SVGStyledTransformableElement.cpp
103950 = 8282a1d85ad097ce4064a5896912bdb211b115e6 already exists! Why are we refetching it?
at /usr/lib/git-core/git-svn line 5210
Died at Tools/Scripts/update-webkit line 158.
If any of these errors are false positives, please file a bug against check-webkit-style.
I'm still not sure why we want to change this. Comment on attachment 120984 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=120984&action=review > Source/WebCore/html/HTMLSelectElement.cpp:257 > + // Setting the value clears the selectedness of all options. > + setSelectedIndex(-1); Does calling setSelectedIndex twice have any additional effect? For example, do we get additional change events? To word this another way, why isn’t this call in the return statement and then again at the end of the function? Comment on attachment 120984 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=120984&action=review >> Source/WebCore/html/HTMLSelectElement.cpp:257 >> + setSelectedIndex(-1); > > Does calling setSelectedIndex twice have any additional effect? For example, do we get additional change events? > > To word this another way, why isn’t this call in the return statement and then again at the end of the function? In this case there should be no extra change event calls, but I like your approach, since that guarantees just one setSelectedIndex() call for any given case. I will do this instead. Without this patch there'd be inconsistency between value and selectedIndex, since both are supposed to clear the selectedness of all options first. Also, when <select> has no selection, value returns ''. I'd expect that setting value to '' would remove any existing selection. Moreover, Alexey and I did some additional testing and found that IE7 and IE8 seem to also follow the spec. So, any sites in which WK behavior would has different from IE's probably will have implemented a separate code path, further preventing any potential breaks in existing web sites. It seems low risk to make this behavior follow the spec, so I will land the patch with Darin's suggested changes. Committed r104864: http://trac.webkit.org/changeset/104864 |