344060 - Setting options.length to a negative integer does not throw an exception

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect, P5)

Description

[David D. Kilzer (ddk)]

MSIE 6 throws an exception when attempting to set options.length to a negative integer.  Discovered while working implementing options.add() for Safari/WebKit.

http://bugzilla.opendarwin.org/show_bug.cgi?id=9179

Note that Firefox does NOT do anything wrong in this case; it ignores the attempt to set the length to a negative value.  It just silently ignores it.

Tested with Firefox 1.5.0.5 rc3 and nightly 3.0a1 build dated Jul 09, 2006.

Comment 1

[David D. Kilzer (ddk)]

Attachment: Test case

Test case.  Prints "FAIL" if no exception is throw, or "PASS" if an exception is thrown.

Comment 2

[Alex Vincent [:WeirdAl]]

I think this bug is INVALID.  Per DOM 2 HTML, the length attribute is supposed to be an unsigned (meaning zero or positive) number.  That's what the IDL for nsIDOMHTMLOptionsCollection says, that's what nsHTMLOptionCollection::SetLength says, and that's what we get.

When you pass in -1, you're going to get the maximum value the number can hold, minus one.

Comment 3

[Alex Vincent [:WeirdAl]]

err, exactly the maximum value (not max - 1).

Comment 4

[David D. Kilzer (ddk)]

(In reply to comment #2)
> I think this bug is INVALID.  Per DOM 2 HTML, the length attribute is supposed
> to be an unsigned (meaning zero or positive) number.  That's what the IDL for
> nsIDOMHTMLOptionsCollection says, that's what nsHTMLOptionCollection::SetLength
> says, and that's what we get.
> 
> When you pass in -1, you're going to get the maximum value the number can hold.

Nope, you don't get that either.  Firefox simply does nothing in this case.  It neither changes the length of the options area, nor throws an exception.

Comment 5

[Firefox Bug Husbandry Bot]

https://bugzilla.mozilla.org/show_bug.cgi?id=1472046

Move all DOM bugs that haven’t been updated in more than 3 years and has no one currently assigned to P5.

If you have questions, please contact :mdaly.

Comment 6

[u597032]

Is this bug still relevant or can it be closed by now?

Comment 7

[David D. Kilzer (ddk)]

(In reply to opi123 from comment #6)

Is this bug still relevant or can it be closed by now?

Did you run the test case on modern web browsers (Safari, Chrome/Edge, Firefox) to see if they agree on compatibility?

If they all "Fail", then there is probably nothing left to do here as MSIE 6 is gone, and modern browsers agree on behavior.

Comment 8

[David D. Kilzer (ddk)]

Safari 15.6 on Mac OS X 12.5 returns: "FAIL No exception thrown".

Comment 9

[u597032]

Safari and Chrome/Edge return "Fail". Only Firefox returns "Pass". This is the opposite of what the bug is for (if I understood that correcty). Is this still a problem then?

Comment 10

[Emilio Cobos Álvarez [:emilio]]

So we follow the WebIDL spec properly, but we have a check for length > 10000, and we throw then. That comes from bug 460713.

Comment 11

[Emilio Cobos Álvarez [:emilio]]

WebKit has the same limit, but just warns, doesn't throw: https://searchfox.org/wubkat/rev/cf0cfbaf1ad7728a145c06e3539170870bbb913f/Source/WebCore/html/HTMLSelectElement.cpp#491

Comment 12

[Robert]

Hi, I would like to tackle this issue.
I've read through the linked github issue and found this related issue: https://github.com/whatwg/html/issues/8250#issuecomment-1263901046
Under 3.v.a it mentions that the spec will be changed and that the expected behaviour should be to clamp the value and print a warning in the console. So I would go ahead and implement this behaviour.

Comment 13

[Robert]

Attachment: Bug 344060 - Change behaviour of huge values for options.length. r=smaug

Setting options.length to a value greater than 10,000 will now result in the length
being clamped to and a warning being output to the browser console.

Comment 14

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/74ea2aedda06
Change behaviour of huge values for options.length. r=emilio

Comment 15

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/36262 for changes under testing/web-platform/tests

Comment 16

[Norisz Fay [:noriszfay]]

https://hg.mozilla.org/mozilla-central/rev/74ea2aedda06

Comment 17

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Upstream PR merged by moz-wptsync-bot