Bug 25191 – HTMLSelectElement doesn't return named options

• Component: DOM
• Status: RESOLVED FIXED

[Dave Moore]

chromium and Safari differ in the way they handle named option values. According to the HTML5 spec named options should be accessible from both the HTMLSelectElement and the HTMLOptionsCollection. If there is more than one option with the same name, all should be returned as a collection. So if you have:

<select id="sl1"><option value="Value" name="test" /></select>

and in JS code you do:
var test = document.getElementById("sl1").test

test should be a single HTMLOptionElement.

If instead you have:

[Dave Moore]

A finger twitch committed early, so I'm restating:

According to the HTML5 spec named options should be accessible from both the
HTMLSelectElement and the HTMLOptionsCollection. If there is more than one
option with the same name, all should be returned as a collection. So if you have:

<select id="sl1"><option value="Value" name="test" /></select>

and in JS code you do:
var test = document.getElementById("sl1").test

test should be a single HTMLOptionElement.

If instead you have:

<select id="sl2"><option value="Value1" name="test" /><option value="Value2" name="test" /></select>

test should be a collection with 2 HTMLOptionElements. 

Neither chromium nor Safari handle both cases correctly. Safari returns both singletons and collections but only for HTMLOptionsCollection (document.getElementById("sl1").options.test). For HTMLSelectElement it always returns null. chromium returns a singleton even when more than one element has the same name for both HTMLSelectElement and HTMLOptionsCollection.

[Dave Moore]

Created attachment 29480 [details]
testcases

[Eric Seidel (no email)]

Comment on attachment 29480 [details]
testcases

No need to ask just a single person to review this.  WebKit generally just marks thigns r=?.  then all reviewers see it, and qualified ones hopefully will review it for you.

This test should be a JS style test.  see examples in fast/js/

or make-test-wrappers.

Any resources directory with a TEMPLATE.html in it, can serve as an example for a js style test (or ojan/dimitri or I can give you more infos in person).

I thought that this test was supposed to fail, no?  You're adding passing results.  Do we not have a bug?

r- for more info needed (since I thought this test was supposed to fail for JSC?)

[Eric Seidel (no email)]

It doesn't have to be a JS test of course. :)  But I think JS tests are much cleaner.

This is how this one would look:

resources/named-options.js:

description("This tests that options elements are accessible by name from both a select element and options collection.");

var sl1 = document.createElement("select");
document.body.appendChild(sl1);
sl1.innerHTML = '<option value="Value" name="test" />';

var sl2 = document.createElement("select");
document.body.appendChild(sl2);
sl1.innerHTML = '<option value="Value1" name="test" /><option value="Value2" name="test" />';

debug("Confirm that the option named "test" is accessible from the select element");
shouldBeEqualToString("sl1.test.value", "Value");

debug("Confirm that the option named "test" is accessible from the options collection");
shouldBeEqualToString("sl1.options.test.value", "Value");

debug("Confirm that both options named "test" are accessible from the select element");
shouldBe("sl2.test.length", "2");
shouldBe("sl2.test[0].value", "Value1");
shouldBe("sl2.test[1].value", "Value2");

debug("Confirm that both options named "test" are accessible from the options collection");
shouldBe("sl2.options.test.length", "2");
shouldBe("sl2.options.test[0].value", "Value1");
shouldBe("sl2.options.test[1].value", "Value2");


I think that kind of test is much easier to read, personally.  Actually, if I were writing this, I would search replace "sl1" with "select1" to make it even more readable.

As is, the test is OK, but I think a js-style test is better.  We really need better documentation on our js-style testing.

That test will produce nicely colored output when you view it in Safari, and will dump as text a bunch of PASS/FAIL messages run under DRT.

[Eric Seidel (no email)]

Created attachment 29649 [details]
JS test example for Dave

 .../HTMLSelectElement/named-options-expected.txt   |   16 ++++++++++
 .../fast/dom/HTMLSelectElement/named-options.html  |   13 ++++++++
 .../dom/HTMLSelectElement/resources/TEMPLATE.html  |   13 ++++++++
 .../HTMLSelectElement/resources/named-options.js   |   31 ++++++++++++++++++++
 4 files changed, 73 insertions(+), 0 deletions(-)

[Eric Seidel (no email)]

So the test (which I hacked together from Dave's test) doesn't pass in Safari TOT or FF 3.  I didn't try other browsers.  Maybe I've written the test wrong, or maybe FF doesn't support this either yet.

I don't think it makes much sense to land a test case w/o fixing the bug in question. :)  It seems WebKit lacks support for this, so we need to add that before the test is of much use.

[Eric Seidel (no email)]

Or rather, the test is only useful currently to show that we don't have support.  (And the non-js test is better at that since sadly we don't currently have an easy solution for making stand-alone js tests... I keep meaning to write a script to do that...)

[Eric Seidel (no email)]

Created attachment 29650 [details]
JS test example for Dave

 .../HTMLSelectElement/named-options-expected.txt   |   21 +++++++++++++
 .../fast/dom/HTMLSelectElement/named-options.html  |   13 ++++++++
 .../dom/HTMLSelectElement/resources/TEMPLATE.html  |   13 ++++++++
 .../HTMLSelectElement/resources/named-options.js   |   31 ++++++++++++++++++++
 4 files changed, 78 insertions(+), 0 deletions(-)

[Eric Seidel (no email)]

Comment on attachment 29649 [details]
JS test example for Dave

I just had some typos in the test itself. :(

[Eric Seidel (no email)]

Created attachment 29651 [details]
LayoutTest with ChangeLog for review

 LayoutTests/ChangeLog                              |   15 ++++++++
 .../HTMLSelectElement/named-options-expected.txt   |   25 ++++++++++++++
 .../fast/dom/HTMLSelectElement/named-options.html  |   13 +++++++
 .../dom/HTMLSelectElement/resources/TEMPLATE.html  |   13 +++++++
 .../HTMLSelectElement/resources/named-options.js   |   35 ++++++++++++++++++++
 5 files changed, 101 insertions(+), 0 deletions(-)

[Eric Seidel (no email)]

Comment on attachment 29650 [details]
JS test example for Dave

Decided to add a couple more tests to the new test and a ChangeLog and post it for review.  Maybe Sam can look at this.

[Eric Seidel (no email)]

This bug needs followup bugs about actually adding this support to WebKit.  Dave, would you be so kind as to file those?

[Eric Seidel (no email)]

Committing to http://svn.webkit.org/repository/webkit/trunk ...
	M	LayoutTests/ChangeLog
	A	LayoutTests/fast/dom/HTMLSelectElement/named-options-expected.txt
	A	LayoutTests/fast/dom/HTMLSelectElement/named-options.html
	A	LayoutTests/fast/dom/HTMLSelectElement/resources/TEMPLATE.html
	A	LayoutTests/fast/dom/HTMLSelectElement/resources/named-options.js
Committed r42716

Dave filed bug 25310 as a followup.  When we fix bug 25310 JSC WebKit will pass this test.  I've been told the V8 bindings have already had this fixed.  :(