Bug 148483 – Range.compareBoundaryPoints() should throw a NotSupportedError for invalid compareHow values

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

[Chris Dumez]

Range.compareBoundaryPoints() should throw a NotSupportedError for invalid compareHow values:
https://dom.spec.whatwg.org/#dom-range-compareboundarypoints (step 1)

Firefox and Chrome conform to the specification already.

W3C test suite:
http://w3c-test.org/dom/ranges/Range-compareBoundaryPoints.html

[Chris Dumez]

Created attachment 259981 [details]
Patch

[Geoffrey Garen]

Comment on attachment 259981 [details]
Patch

r=me

[Chris Dumez]

Created attachment 260091 [details]
Patch

[Chris Dumez]

Comment on attachment 260091 [details]
Patch

Clearing flags on attachment: 260091

Committed r189062: <http://trac.webkit.org/changeset/189062>

[Chris Dumez]

All reviewed patches have been landed.  Closing bug.

[Darin Adler]

Comment on attachment 260091 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=260091&action=review

> LayoutTests/fast/dom/Range/compareBoundaryPoints-compareHow-exception-expected.txt:10
> +PASS range.compareBoundaryPoints(65536, sourceRange) is -1

This is the desired behavior? Why not an exception in this case?

[Chris Dumez]

Comment on attachment 260091 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=260091&action=review

>> LayoutTests/fast/dom/Range/compareBoundaryPoints-compareHow-exception-expected.txt:10
>> +PASS range.compareBoundaryPoints(65536, sourceRange) is -1
> 
> This is the desired behavior? Why not an exception in this case?

Because 65536 wraps to 0, which is the value of the START_TO_START constant. compareHow is an unsigned short so it is in the range [0, 65535] as per:
https://heycam.github.io/webidl/#idl-unsigned-short

The unsigned short conversion is documented at:
https://heycam.github.io/webidl/#es-unsigned-short
which says to call ToUint16(x) in this case:
http://people.mozilla.org/~jorendorff/es6-draft.html#sec-touint16 (which specifies the wrapping around behavior)

We would only throw a TypeError if the IDL specified [EnforceRange]:
https://heycam.github.io/webidl/#EnforceRange

[Chris Dumez]

Comment on attachment 260091 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=260091&action=review

> LayoutTests/fast/dom/Range/compareBoundaryPoints-compareHow-exception.html:20
> +shouldBe("range.compareBoundaryPoints(65536, sourceRange)", "-1"); // 65536 should wrap around to 0.

By the way, I documented the wrapping around to 0 here already.

[Chris Dumez]

Comment on attachment 260091 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=260091&action=review

>>> LayoutTests/fast/dom/Range/compareBoundaryPoints-compareHow-exception-expected.txt:10
>>> +PASS range.compareBoundaryPoints(65536, sourceRange) is -1
>> 
>> This is the desired behavior? Why not an exception in this case?
> 
> Because 65536 wraps to 0, which is the value of the START_TO_START constant. compareHow is an unsigned short so it is in the range [0, 65535] as per:
> https://heycam.github.io/webidl/#idl-unsigned-short
> 
> The unsigned short conversion is documented at:
> https://heycam.github.io/webidl/#es-unsigned-short
> which says to call ToUint16(x) in this case:
> http://people.mozilla.org/~jorendorff/es6-draft.html#sec-touint16 (which specifies the wrapping around behavior)
> 
> We would only throw a TypeError if the IDL specified [EnforceRange]:
> https://heycam.github.io/webidl/#EnforceRange

This layout test is also passing in the latest Firefox and Chrome.