[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.
Created attachment 271990 [details] Patch
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.
Comment on attachment 271990 [details] Patch Clearing flags on attachment: 271990 Committed r197014: <http://trac.webkit.org/changeset/197014>
All reviewed patches have been landed. Closing bug.