Bug 41949 – Implement SVG attribute case mapping for HTMLTreeBuilder

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Eric Seidel (no email)]

Implement SVG attribute case mapping for HTMLTreeBuilder

[Eric Seidel (no email)]

Created attachment 61032 [details]
Patch

[Eric Seidel (no email)]

Created attachment 61073 [details]
Patch

[Adam Barth]

Comment on attachment 61073 [details]
Patch

WebCore/html/HTMLTreeBuilder.cpp:742
 +      for (unsigned x = 0; x < attributes->length(); ++x) {
I'd prefer "i" here, but whatever.

WebCore/html/HTMLTreeBuilder.cpp:743
 +          Attribute* attribute = attributes->attributeItem(x);
I'm not sure what the internal structure of the attributes data structure is.  Is it efficient to index each time, or is there an iterator API we should be using.

[Adam Barth]

+darin in case he has feedback in the pattern of introducing parser* APIs that might not be safe for general use by the rest of the code.

[Eric Seidel (no email)]

Comment on attachment 61073 [details]
Patch

Its a vector. :)

[Darin Adler]

(In reply to comment #4)
> +darin in case he has feedback in the pattern of introducing parser* APIs that might not be safe for general use by the rest of the code.

Thanks for cc'ing me.

I think it’s good to give the function a clear name the way you have, Eric. But also it would be best if you could state in some clear way what is "unsafe" about using this in other code. A more concrete comment would help people understand what the real issue is.

[Eric Seidel (no email)]

Certainly.  I'm actually not 100% sure it's unsafe, but it seems possibly unsafe if others have a hash of Attributes.  I don't really know enough about the attribute system to know if this is safe or not.  I presumed it was not since QualifiedName has no setLocalName accessor, and Attribute seemed to have explicitly avoided it in the past (or at least forgotten to add one).

[Eric Seidel (no email)]

Talked to hyatt.  parserSetLocalName() is only ssafe up until parseMappedAttributes.

[Darin Adler]

(In reply to comment #8)
> Talked to hyatt.  parserSetLocalName() is only ssafe up until parseMappedAttributes.

One possible way to reflect that in the code is to devote a debug-only boolean to this and assert that parseMappedAttributes didn't happen yet.

[Eric Seidel (no email)]

Committed r62994: <http://trac.webkit.org/changeset/62994>