Bug 154573 – [Reflected] IDL attributes of integer types should use HTML rules for parsing integers

• Status: RESOLVED
• Resolution: FIXED
• Component: Bindings

[Chris Dumez]

[Reflected] IDL attributes of integer types should use HTML rules for parsing integers:
- https://html.spec.whatwg.org/#reflecting-content-attributes-in-idl-attributes

Those rules are defined here:
- https://html.spec.whatwg.org/#rules-for-parsing-integers
- https://html.spec.whatwg.org/#rules-for-parsing-non-negative-integers

We already have an implementation for parsing HTML integers but our reflected attributes currently use WTFString::toInt() / toUint() instead.

[Chris Dumez]

Created attachment 271990 [details]
Patch

[Darin Adler]

Comment on attachment 271990 [details]
Patch

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

> Source/WebCore/dom/Element.cpp:2769
> +    parseHTMLInteger(getAttribute(attributeName), value);

Kind of sad that this doesn’t use fastGetAttribute.

> Source/WebCore/dom/Element.cpp:2781
> +    parseHTMLNonNegativeInteger(getAttribute(attributeName), value);

Kind of sad that this doesn’t use fastGetAttribute.

> Source/WebCore/html/parser/HTMLParserIdioms.cpp:156
>  static bool parseHTMLIntegerInternal(const CharacterType* position, const CharacterType* end, int& value)

Do we have tests for parseHTMLInteger and parseHTMLNonNegativeInteger?

> Source/WebCore/html/parser/HTMLParserIdioms.cpp:195
> +    StringBuilder cleanCharacters;
> +    if (!isPositive)
> +        cleanCharacters.append('-');
>      while (position < end) {
>          if (!isASCIIDigit(*position))
>              break;
> -        digits.append(*position++);
> +        cleanCharacters.append(*position++);
>      }

This is needlessly inefficient, doing heap access for no reason. Given that we are only looking at ASCII digits with a single possible sign prefix, why use a StringBuilder? Why not just use a Vector<LChar, 32> or something like that? We could also use reserveInitialCapacity and uncheckedAppend. And there would be no need for an 8-bit and 16-bit branch below.

[WebKit Commit Bot]

Comment on attachment 271990 [details]
Patch

Clearing flags on attachment: 271990

Committed r197014: <http://trac.webkit.org/changeset/197014>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.