[webkit-reviews] review granted: [Bug 66423] libxml2 fragment parser loses prefix namespaces : [Attachment 105818] Patch
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Wed Aug 31 13:11:09 PDT 2011
Eric Seidel <eric at webkit.org> has granted Jeffrey Pfau <jeffrey at endrift.com>'s
request for review:
Bug 66423: libxml2 fragment parser loses prefix namespaces
https://bugs.webkit.org/show_bug.cgi?id=66423
Attachment 105818: Patch
https://bugs.webkit.org/attachment.cgi?id=105818&action=review
------- Additional Comments from Eric Seidel <eric at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=105818&action=review
The patch looks great. I would perfer you landed an updated test if you have a
minute.
> LayoutTests/ChangeLog:1
> +2011-08-31 Jeffrey Pfau <jeffrey at endrift.com>
apple.com?
> LayoutTests/fast/parser/innerhtml-with-prefixed-elements.xhtml:10
> +<html xmlns="http://www.w3.org/1999/xhtml">
> + <head>
> + <title>Namespace chaining</title>
> + </head>
> + <body>
> + <div id="d" xmlns:svg="http://www.w3.org/2000/svg">
> + TEST FAILED
> + </div>
> + <script type="text/javascript"><![CDATA[
> + var div = document.getElementById("d");
I think normally we use 4 space indent.
> LayoutTests/fast/parser/innerhtml-with-prefixed-elements.xhtml:11
> + div.innerHTML = "<svg:svg width='20'
height='20'><svg:defs id='defs'><svg:text>TEST
FAILED</svg:text></svg:defs><svg:circle cx='10' cy='10' r='4'
id='c'/></svg:svg>";
Would be much better to use a 100x100 green square to show success (it's an old
convention from the w3c CSS working group test suite).
> Source/WebCore/ChangeLog:1
> +2011-08-31 Jeffrey Pfau <jeffrey at endrift.com>
apple.com?
> Source/WebCore/xml/parser/XMLDocumentParserLibxml2.cpp:614
> - for (Element* element = elemStack.last(); !elemStack.isEmpty();
elemStack.removeLast()) {
> + for (; !elemStack.isEmpty(); elemStack.removeLast()) {
> + Element* element = elemStack.last();
Wow. I wonder how many other bad for loops like this we have. Some day c++
may have a foreach. :)
More information about the webkit-reviews
mailing list