Bug 71669 – Fix wrong test results of fast/js/custom-constructors.html

• Status: RESOLVED
• Resolution: FIXED
• Component: DOM

[Kentaro Hara]

Some test results in custom-constructors.html are wrong: 

wrong: shouldBeEqualToString("new Option(undefined).innerText", "");
correct: shouldBeEqualToString("new Option(undefined).innerText", "undefined");

wrong: shouldBeEqualToString("new Option('somedata', undefined).value", "somedata");
correct: shouldBeEqualToString("new Option('somedata', undefined).value", "undefined");

According to the HTMLOptionElement spec (http://dev.w3.org/html5/spec/the-button-element.html#the-option-element), 'label' and 'value' do not have [TreatUndefinedAs=EmptyString]. Thus, a value undefined should be converted to a string "undefined", following the step 3 of the IDL conversion spec (http://dev.w3.org/2006/webapi/WebIDL/#es-DOMString).

[Kentaro Hara]

Created attachment 113848 [details]
Patch

[WebKit Review Bot]

Comment on attachment 113848 [details]
Patch

Attachment 113848 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/10337757

New failing tests:
fast/dom/element-attribute-js-null.html

[Kentaro Hara]

Created attachment 113877 [details]
Patch

[Ojan Vafai]

What do IE, Gecko and Opera do? If we all match, then it might make sense for the spec to change. If WebKit's the only one with this behavior, then this patch seems right to me.

[Adam Barth]

I agree with Ojan that we should see what other browsers do to determine whether the bug is in WebKit or the spec.

[Kentaro Hara]

(In reply to comment #5)
> I agree with Ojan that we should see what other browsers do to determine whether the bug is in WebKit or the spec.

Thank you, Ojan and Adam!

Firefox 7.0.1:
new Option(undefined).text => "undefined"
new Option(null).text => "null"
new Option('aaa', undefined).value => "undefined"
new Option('aaa', null).value => "null"
o = new Option(); o.label = undefined; o.label => "undefined"
o = new Option(); o.label = null; o.label => ""

Opera 11.52:
new Option(undefined).text => "undefined"
new Option(null).text => ""
new Option('aaa', undefined).value => "undefined"
new Option('aaa', null).value => ""
o = new Option(); o.label = undefined; o.label => "undefined"
o = new Option(); o.label = null; o.label => ""

Chrome 16.0.912.21 beta:
new Option(undefined).text => ""
new Option(null).text => "null"
new Option('aaa', undefined).value => "aaa"
new Option('aaa', null).value => "null"
o = new Option(); o.label = undefined; o.label => "undefined"
o = new Option(); o.label = null; o.label => ""

The spec:
new Option(undefined).text => "undefined"
new Option(null).text => "null"
new Option('aaa', undefined).value => "undefined"
new Option('aaa', null).value => "null"
o = new Option(); o.label = undefined; o.label => "undefined"
o = new Option(); o.label = null; o.label => "null"

I do not have an IE environment now. Could anyone try it?

All are different:-) Should we follow the spec in this case?

[Kentaro Hara]

Created attachment 113916 [details]
test case HTML

Attached a test case. Could anyone open this HTML in IE and paste the result here?

[Kentaro Hara]

Created attachment 113917 [details]
Patch

[Darin Adler]

Comment on attachment 113917 [details]
Patch

Looks fine. Did we double check that the other major browsers match the spec?

[Kentaro Hara]

Darin, thanks!

IE:
new Option(undefined).text => ""
new Option(null).text => "null"
new Option('aaa', undefined).value => ""
new Option('aaa', null).value => "null"
o = new Option(); o.label = undefined; o.label => "undefined"
o = new Option(); o.label = null; o.label => "null"

IE's behavior is also different from any other browsers'. With these observations, I think that following the spec makes sense.

[WebKit Review Bot]

Comment on attachment 113917 [details]
Patch

Clearing flags on attachment: 113917

Committed r99510: <http://trac.webkit.org/changeset/99510>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.