Bug 64305 – Move XML errors handling out from XMLDocumentParser (refactoring).

• Component: XML
• Status: RESOLVED FIXED

[Vsevolod Vlasov]

Move XML errors handling out from XMLDocumentParser.

This is a part of http://webk.it/55233 (XSL transformations do not handle errors).

[Vsevolod Vlasov]

Created attachment 100350 [details]
Patch

[Adam Barth]

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.

[Vsevolod Vlasov]

> > 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.

[Vsevolod Vlasov]

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.

[Early Warning System Bot]

Comment on attachment 100350 [details]
Patch

Attachment 100350 [details] did not pass qt-ews (qt):
Output: http://queues.webkit.org/results/9010580

[Vsevolod Vlasov]

Created attachment 100354 [details]
Patch (comments addressed + qt build fix)

[Adam Barth]

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.)

[Adam Barth]

Neither of those things are your fault, but the StringBuilder one should be easy to fix while you're here.

[Vsevolod Vlasov]

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)

[Vsevolod Vlasov]

Committed r91008: <http://trac.webkit.org/changeset/91008>