Bug 154771 – Fix the behavior of reflecting IDL attributes of type unsigned long

• Status: RESOLVED
• Resolution: FIXED
• Component: History

[Chris Dumez]

Fix the behavior of reflecting IDL attributes of type unsigned long to align with the specification:
- https://html.spec.whatwg.org/multipage/infrastructure.html#reflecting-content-attributes-in-idl-attributes
- https://html.spec.whatwg.org/multipage/infrastructure.html#rules-for-parsing-non-negative-integers

There are several issues with the current implementation:
- Upon getting, the value returned must be in the range 0 to 2147483647. Otherwise, we must return the default value (0 unless specified otherwise). We currently return values in the range 0 to 4294967295 instead.
- Upon setting, we must set the content attribute to the default value (0 unless specified otherwise) if the input value is not in the range 0 to 2147483647. We currently allow values in the range 0 to 4294967295 instead.
- "-0" is not recognized as a valid unsigned integer

[Chris Dumez]

Firefox and Chrome match the specification.

[Chris Dumez]

Created attachment 272399 [details]
Patch

[Ryosuke Niwa]

Comment on attachment 272399 [details]
Patch

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

> Source/WebCore/html/parser/HTMLParserIdioms.h:123
> +    ASSERT(value > 0 && value <= 2147483647);

Can we use 0x7FFFFFFF instead and define a static const somewhere?
e.g. static const MaxHTMLNonNegativeNumber = 0x7FFFFFFF.

[Chris Dumez]

(In reply to comment #3)
> Comment on attachment 272399 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=272399&action=review
> 
> > Source/WebCore/html/parser/HTMLParserIdioms.h:123
> > +    ASSERT(value > 0 && value <= 2147483647);
> 
> Can we use 0x7FFFFFFF instead and define a static const somewhere?
> e.g. static const MaxHTMLNonNegativeNumber = 0x7FFFFFFF.

I find 2147483647 a lot more readable than 0x7FFFFFFF personally. It also matches what is in the HTML and the Web IDL specs.

[Ryosuke Niwa]

(In reply to comment #4)
> (In reply to comment #3)
> > Comment on attachment 272399 [details]
> > Patch
> > 
> > View in context:
> > https://bugs.webkit.org/attachment.cgi?id=272399&action=review
> > 
> > > Source/WebCore/html/parser/HTMLParserIdioms.h:123
> > > +    ASSERT(value > 0 && value <= 2147483647);
> > 
> > Can we use 0x7FFFFFFF instead and define a static const somewhere?
> > e.g. static const MaxHTMLNonNegativeNumber = 0x7FFFFFFF.
> 
> I find 2147483647 a lot more readable than 0x7FFFFFFF personally. It also
> matches what is in the HTML and the Web IDL specs.

Really!? 2147483647 doesn't tell me anything about why it's significant whereas 0x7FFFFFFF would tell us exactly why that value is picked.

[Chris Dumez]

Created attachment 272405 [details]
Patch

[Chris Dumez]

Comment on attachment 272405 [details]
Patch

Clearing flags on attachment: 272405

Committed r197237: <http://trac.webkit.org/changeset/197237>

[Chris Dumez]

All reviewed patches have been landed.  Closing bug.

[Darin Adler]

(In reply to comment #4)
> I find 2147483647 a lot more readable than 0x7FFFFFFF personally. It also
> matches what is in the HTML and the Web IDL specs.

I am not sure what you mean by readable, but I think that’s not really possible. For example, what if I wrote this:

    2147383647

Would you be able to spot that value is not 2^31-1? It’s true that the same problem can happen with:

    0x7FFFFFF

But I, at least, can see the difference, whereas with the decimal number it’s just always unsafe, could have one digit wrong and we’d never see it.