Summary: | Move XML errors handling out from XMLDocumentParser (refactoring). | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Vsevolod Vlasov <vsevik> | ||||||
Component: | XML | Assignee: | Vsevolod Vlasov <vsevik> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Normal | CC: | abarth, jeffrey+webkit | ||||||
Priority: | P2 | ||||||||
Version: | 528+ (Nightly build) | ||||||||
Hardware: | Unspecified | ||||||||
OS: | Unspecified | ||||||||
Bug Depends on: | |||||||||
Bug Blocks: | 55233 | ||||||||
Attachments: |
|
Description
Vsevolod Vlasov
2011-07-11 12:19:02 PDT
Created attachment 100350 [details]
Patch
Comment on attachment 100350 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=100350&action=review Sounds like a good idea. I didn't verify that the moved code was identical, but I did find at least one difference, which makes me feel like I need to look more carefully in order to review the patch thoroughly. > Source/WebCore/xml/XMLErrors.cpp:72 > + case tip: > + break; Where did this case come from? It doesn't appear to have existed before. > > Source/WebCore/xml/XMLErrors.cpp:72
> > + case tip:
> > + break;
>
> Where did this case come from? It doesn't appear to have existed before.
I just added that for myself and forgot to remove, thanks for spotting it.
I'll fix that together with other comments if any.
Actually this "tip" type is coming from libxml2 and will be used later (as it is used currently currently for XSLT error handling), but this is out of scope of this bug. Comment on attachment 100350 [details] Patch Attachment 100350 [details] did not pass qt-ews (qt): Output: http://queues.webkit.org/results/9010580 Created attachment 100354 [details]
Patch (comments addressed + qt build fix)
Comment on attachment 100354 [details] Patch (comments addressed + qt build fix) View in context: https://bugs.webkit.org/attachment.cgi?id=100354&action=review > Source/WebCore/xml/XMLErrors.cpp:85 > + RefPtr<Element> h3 = doc->createElement(h3Tag, false); > + reportElement->appendChild(h3.get(), ec); Can there be mutation event registered in doc to screw us over? > Source/WebCore/xml/XMLErrors.h:59 > + String m_errorMessages; Consider using StringBuilder instead. (See Darin's recent post on webkit-dev.) Neither of those things are your fault, but the StringBuilder one should be easy to fix while you're here. I'll fix StringBuilder thing before committing. I have also filed https://bugs.webkit.org/show_bug.cgi?id=64532 for the other problem (with an example where it fails) Committed r91008: <http://trac.webkit.org/changeset/91008> |