Summary: | Implement SVG attribute case mapping for HTMLTreeBuilder | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Eric Seidel (no email) <eric> | ||||||
Component: | New Bugs | Assignee: | Eric Seidel (no email) <eric> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Normal | CC: | abarth, darin, hyatt | ||||||
Priority: | P2 | ||||||||
Version: | 528+ (Nightly build) | ||||||||
Hardware: | Other | ||||||||
OS: | OS X 10.5 | ||||||||
Bug Depends on: | |||||||||
Bug Blocks: | 41123 | ||||||||
Attachments: |
|
Description
Eric Seidel (no email)
2010-07-09 04:10:45 PDT
Created attachment 61032 [details]
Patch
Created attachment 61073 [details]
Patch
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.
+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. Comment on attachment 61073 [details]
Patch
Its a vector. :)
(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. 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). Talked to hyatt. parserSetLocalName() is only ssafe up until parseMappedAttributes. (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. Committed r62994: <http://trac.webkit.org/changeset/62994> |