Bug 67233 – Setting value on a select element to a non existing option value should clear selection

• Component: Forms
• Status: RESOLVED FIXED

[Erik Arvidsson]

Given:

<select>
  <option value="a">A</option>
  <option value="b">B</option>
</select>

<script>
var sel = document.querySelector('select');
sel.value = 'x'
assert(sel.selectedIndex === -1)
</script>

The spec says: "On setting, the value attribute must set the selectedness of all the option elements in the list of options to false, and then the first option element in the list of options, in tree order, whose value is equal to the given new value, if any, must have its selectedness set to true."

This means that we should unselect all and then select first option that matches the value.

[Alexey Proskuryakov]

This fails in WebKit and in Firefox, so sounds like a mistake in the spec.

[Rafael Weinstein]

IE9 follows the spec. FWIW (probably not much), the spec seems like the right (desirable) behavior to me.

[Radar WebKit Bug Importer]

<rdar://problem/10057159>

[Alexey Proskuryakov]

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?

[Jon Lee]

Created attachment 120984 [details]
Patch

[WebKit Review Bot]

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.

[Alexey Proskuryakov]

I'm still not sure why we want to change this.

[Darin Adler]

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?

[Jon Lee]

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.

[Jon Lee]

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.

[Jon Lee]

Committed r104864: http://trac.webkit.org/changeset/104864