Title: [252230] trunk
Revision
252230
Author
commit-qu...@webkit.org
Date
2019-11-07 18:52:40 -0800 (Thu, 07 Nov 2019)

Log Message

Default NamepaceURI must be gotten from the topmost parent before the SVG <foreignObject>
https://bugs.webkit.org/show_bug.cgi?id=203868

Patch by Said Abou-Hallawa <sabouhall...@apple.com> on 2019-11-07
Reviewed by Ryosuke Niwa.

Source/WebCore:

Ensure that we don't cross boundaries from HTML to SVG when traversing
the tree of nodes upward. We need to stop at the foreignObject if it is
one of the ancestors of the contextElement.

Tests: svg/foreignObject/foreign-object-dynamic-parsing.svg

* html/HTMLTableCellElement.cpp:
(WebCore::HTMLTableCellElement::HTMLTableCellElement):
This assertion should not fire if the tag has a prefix like <h:th> or
<h:td> where 'h' is a defined namespace.

* xml/parser/XMLDocumentParser.cpp:
(WebCore::XMLDocumentParser::parseDocumentFragment):
Stop at the first SVG <foreignObject> ancestor when calculating the
defaultNamespaceURI.

* xml/parser/XMLDocumentParser.h:
* xml/parser/XMLDocumentParserLibxml2.cpp:
(WebCore::XMLDocumentParser::XMLDocumentParser):

(WebCore::XMLDocumentParser::startElementNs):
We need to special case setting the namespaceURI of the SVGElmenets. The
defaultNamespaceURI can be wrong for them if the context element is an
HTML element, <div> for example, and the innerHTML is set to something
like: '<svg><rect/></svg>'.

LayoutTests:

* svg/foreignObject/foreign-object-dynamic-parsing-expected.svg: Added.
* svg/foreignObject/foreign-object-dynamic-parsing.svg: Added.

Modified Paths

Added Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (252229 => 252230)


--- trunk/LayoutTests/ChangeLog	2019-11-08 02:46:09 UTC (rev 252229)
+++ trunk/LayoutTests/ChangeLog	2019-11-08 02:52:40 UTC (rev 252230)
@@ -1,3 +1,13 @@
+2019-11-07  Said Abou-Hallawa  <sabouhall...@apple.com>
+
+        Default NamepaceURI must be gotten from the topmost parent before the SVG <foreignObject>
+        https://bugs.webkit.org/show_bug.cgi?id=203868
+
+        Reviewed by Ryosuke Niwa.
+
+        * svg/foreignObject/foreign-object-dynamic-parsing-expected.svg: Added.
+        * svg/foreignObject/foreign-object-dynamic-parsing.svg: Added.
+
 2019-11-07  Chris Dumez  <cdu...@apple.com>
 
         TestController may reuse a view that used window.open(), which prevents process-swapping and causes flakiness

Added: trunk/LayoutTests/svg/foreignObject/foreign-object-dynamic-parsing-expected.svg (0 => 252230)


--- trunk/LayoutTests/svg/foreignObject/foreign-object-dynamic-parsing-expected.svg	                        (rev 0)
+++ trunk/LayoutTests/svg/foreignObject/foreign-object-dynamic-parsing-expected.svg	2019-11-08 02:52:40 UTC (rev 252230)
@@ -0,0 +1,34 @@
+<svg xmlns="http://www.w3.org/2000/svg">
+    <foreignObject x="10" y="10" width="100" height="100">
+        <div xmlns="http://www.w3.org/1999/xhtml">
+            <h2>ABC</h2>
+            <table style="border: 1px solid black;">
+                <thead>
+                    <tr>
+                        <th>A</th>
+                        <th>B</th>
+                        <th>C</th>
+                    </tr>
+                </thead>
+            </table>
+        </div>
+    </foreignObject>
+    <foreignObject x="120" y="10" width="100" height="100">
+        <div xmlns="http://www.w3.org/1999/xhtml">
+            <h2>DEF</h2>
+            <table style="border: 1px solid black;">
+                <thead>
+                    <tr>
+                        <th>D</th>
+                        <th>E</th>
+                        <th>F</th>
+                    </tr>
+                </thead>
+            </table>
+        </div>
+    </foreignObject>
+    <rect x="10" y="120" width="100" height="100" fill="green"/>
+    <rect x="120" y="120" width="100" height="100" fill="green"/>
+    <rect x="10" y="230" width="100" height="100" fill="green"/>
+    <rect x="120" y="230" width="100" height="100" fill="green"/>
+</svg>

Added: trunk/LayoutTests/svg/foreignObject/foreign-object-dynamic-parsing.svg (0 => 252230)


--- trunk/LayoutTests/svg/foreignObject/foreign-object-dynamic-parsing.svg	                        (rev 0)
+++ trunk/LayoutTests/svg/foreignObject/foreign-object-dynamic-parsing.svg	2019-11-08 02:52:40 UTC (rev 252230)
@@ -0,0 +1,72 @@
+<svg xmlns="http://www.w3.org/2000/svg" xmlns:h="http://www.w3.org/1999/xhtml" xmlns:s="http://www.w3.org/2000/svg">
+    <script><![CDATA[
+        function createSVGElement(name, attrs, parentElement) {
+            const svgNamespace = "http://www.w3.org/2000/svg";
+            var element = document.createElementNS(svgNamespace, name);
+
+            for (var key in attrs)
+                element.setAttribute(key, attrs[key]);
+
+            parentElement.appendChild(element);
+            return element;
+        }
+
+        function createHTMLElement(name) {
+            const xhtmlNamespace = "http://www.w3.org/1999/xhtml";
+            return document.createElementNS(xhtmlNamespace, name);
+        }
+
+        var root = document.documentElement;
+
+        var foreignObject1 = createSVGElement("foreignObject", { x: 10, y: 10, width: 100, height: 100 }, root);
+        foreignObject1.appendChild(createHTMLElement("div"));
+        foreignObject1.lastChild.innerHTML =
+            "<h2>ABC</h2>" +
+            "<table style='border: 1px solid black;'>" +
+                "<thead>" +
+                    "<tr>" +
+                        "<th>A</th>" +
+                        "<th>B</th>" +
+                        "<th>C</th>" +
+                    "</tr>" +
+                "</thead>" +
+            "</table>";
+
+
+        var foreignObject2 = createSVGElement("foreignObject", { x: 120, y: 10, width: 100, height: 100 }, root);
+        foreignObject2.appendChild(createHTMLElement("h:div"));
+        foreignObject2.lastChild.innerHTML =
+            "<h:h2>DEF</h:h2>" +
+            "<h:table style='border: 1px solid black;'>" +
+                "<h:thead>" +
+                    "<h:tr>" +
+                        "<h:th>D</h:th>" +
+                        "<h:th>E</h:th>" +
+                        "<h:th>F</h:th>" +
+                    "</h:tr>" +
+                "</h:thead>" +
+            "</h:table>";
+
+        var foreignObject3 = createSVGElement("foreignObject", { x: 10, y: 120, width: 100, height: 100 }, root);
+        foreignObject3.appendChild(createHTMLElement("h:div"));
+        foreignObject3.lastChild.innerHTML = 
+            "<svg>" +
+                "<rect width='100' height='100' fill='green'/>" +
+            "</svg>";
+
+        var foreignObject4 = createSVGElement("foreignObject", { x: 120, y: 120, width: 100, height: 100 }, root);
+        foreignObject4.appendChild(createHTMLElement("h:div"));
+        foreignObject4.lastChild.innerHTML = 
+            "<s:svg>" +
+                "<s:rect width='100' height='100' fill='green'/>" +
+            "</s:svg>";
+
+        var svg1 = createSVGElement("svg", { }, root);
+        var g1 = createSVGElement("g", { transform: 'translate(10, 230)' }, svg1);
+        g1.innerHTML = "<rect width='100' height='100' fill='green'/>";
+
+        var svg2 = createSVGElement("s:svg", { }, root);
+        var g2 = createSVGElement("s:g", { transform: 'translate(120, 230)' }, svg2);
+        g2.innerHTML = "<s:rect width='100' height='100' fill='green'/>";
+    ]]></script>
+</svg>

Modified: trunk/Source/WebCore/ChangeLog (252229 => 252230)


--- trunk/Source/WebCore/ChangeLog	2019-11-08 02:46:09 UTC (rev 252229)
+++ trunk/Source/WebCore/ChangeLog	2019-11-08 02:52:40 UTC (rev 252230)
@@ -1,3 +1,36 @@
+2019-11-07  Said Abou-Hallawa  <sabouhall...@apple.com>
+
+        Default NamepaceURI must be gotten from the topmost parent before the SVG <foreignObject>
+        https://bugs.webkit.org/show_bug.cgi?id=203868
+
+        Reviewed by Ryosuke Niwa.
+
+        Ensure that we don't cross boundaries from HTML to SVG when traversing
+        the tree of nodes upward. We need to stop at the foreignObject if it is
+        one of the ancestors of the contextElement.
+
+        Tests: svg/foreignObject/foreign-object-dynamic-parsing.svg
+
+        * html/HTMLTableCellElement.cpp:
+        (WebCore::HTMLTableCellElement::HTMLTableCellElement):
+        This assertion should not fire if the tag has a prefix like <h:th> or
+        <h:td> where 'h' is a defined namespace.
+
+        * xml/parser/XMLDocumentParser.cpp:
+        (WebCore::XMLDocumentParser::parseDocumentFragment):
+        Stop at the first SVG <foreignObject> ancestor when calculating the
+        defaultNamespaceURI.
+
+        * xml/parser/XMLDocumentParser.h:
+        * xml/parser/XMLDocumentParserLibxml2.cpp:
+        (WebCore::XMLDocumentParser::XMLDocumentParser):
+
+        (WebCore::XMLDocumentParser::startElementNs):
+        We need to special case setting the namespaceURI of the SVGElmenets. The
+        defaultNamespaceURI can be wrong for them if the context element is an
+        HTML element, <div> for example, and the innerHTML is set to something
+        like: '<svg><rect/></svg>'.
+
 2019-11-07  Chris Dumez  <cdu...@apple.com>
 
         Use ActiveDOMObject::queueTaskKeepingObjectAlive() in DOMCache

Modified: trunk/Source/WebCore/html/HTMLTableCellElement.cpp (252229 => 252230)


--- trunk/Source/WebCore/html/HTMLTableCellElement.cpp	2019-11-08 02:46:09 UTC (rev 252229)
+++ trunk/Source/WebCore/html/HTMLTableCellElement.cpp	2019-11-08 02:52:40 UTC (rev 252230)
@@ -57,7 +57,7 @@
 HTMLTableCellElement::HTMLTableCellElement(const QualifiedName& tagName, Document& document)
     : HTMLTablePartElement(tagName, document)
 {
-    ASSERT(tagName == thTag || tagName == tdTag);
+    ASSERT(hasLocalName(thTag->localName()) || hasLocalName(tdTag->localName()));
 }
 
 unsigned HTMLTableCellElement::colSpan() const

Modified: trunk/Source/WebCore/xml/parser/XMLDocumentParser.cpp (252229 => 252230)


--- trunk/Source/WebCore/xml/parser/XMLDocumentParser.cpp	2019-11-08 02:46:09 UTC (rev 252229)
+++ trunk/Source/WebCore/xml/parser/XMLDocumentParser.cpp	2019-11-08 02:52:40 UTC (rev 252230)
@@ -31,6 +31,7 @@
 #include "Document.h"
 #include "DocumentFragment.h"
 #include "DocumentType.h"
+#include "ElementAncestorIterator.h"
 #include "Frame.h"
 #include "FrameLoader.h"
 #include "FrameView.h"
@@ -43,6 +44,7 @@
 #include "ResourceError.h"
 #include "ResourceRequest.h"
 #include "ResourceResponse.h"
+#include "SVGForeignObjectElement.h"
 #include "SVGNames.h"
 #include "SVGStyleElement.h"
 #include "ScriptElement.h"
@@ -50,6 +52,7 @@
 #include "StyleScope.h"
 #include "TextResourceDecoder.h"
 #include "TreeDepthLimit.h"
+#include "XMLNSNames.h"
 #include <wtf/Ref.h>
 #include <wtf/Threading.h>
 #include <wtf/Vector.h>
@@ -269,7 +272,28 @@
         return true;
     }
 
-    auto parser = XMLDocumentParser::create(fragment, contextElement, parserContentPolicy);
+    HashMap<AtomString, AtomString> prefixToNamespaceMap;
+    AtomString defaultNamespaceURI;
+    bool stopLookingForDefaultNamespaceURI = false;
+    
+    for (auto& element : elementLineage(contextElement)) {
+        if (is<SVGForeignObjectElement>(element))
+            stopLookingForDefaultNamespaceURI = true;
+        else if (!stopLookingForDefaultNamespaceURI)
+            defaultNamespaceURI = element.namespaceURI();
+
+        if (!element.hasAttributes())
+            continue;
+
+        for (const Attribute& attribute : element.attributesIterator()) {
+            if (attribute.prefix() == xmlnsAtom())
+                prefixToNamespaceMap.set(attribute.localName(), attribute.value());
+            else if (!stopLookingForDefaultNamespaceURI && attribute.prefix() == xmlnsAtom())
+                defaultNamespaceURI = attribute.value();
+        }
+    }
+
+    auto parser = XMLDocumentParser::create(fragment, WTFMove(prefixToNamespaceMap), defaultNamespaceURI, parserContentPolicy);
     bool wellFormed = parser->appendFragmentSource(chunk);
     // Do not call finish(). The finish() and doEnd() implementations touch the main document and loader and can cause crashes in the fragment case.
     parser->detach(); // Allows ~DocumentParser to assert it was detached before destruction.

Modified: trunk/Source/WebCore/xml/parser/XMLDocumentParser.h (252229 => 252230)


--- trunk/Source/WebCore/xml/parser/XMLDocumentParser.h	2019-11-08 02:46:09 UTC (rev 252229)
+++ trunk/Source/WebCore/xml/parser/XMLDocumentParser.h	2019-11-08 02:52:40 UTC (rev 252230)
@@ -67,9 +67,9 @@
     {
         return adoptRef(*new XMLDocumentParser(document, view));
     }
-    static Ref<XMLDocumentParser> create(DocumentFragment& fragment, Element* element, ParserContentPolicy parserContentPolicy)
+    static Ref<XMLDocumentParser> create(DocumentFragment& fragment, HashMap<AtomString, AtomString>&& prefixToNamespaceMap, const AtomString& defaultNamespaceURI, ParserContentPolicy parserContentPolicy)
     {
-        return adoptRef(*new XMLDocumentParser(fragment, element, parserContentPolicy));
+        return adoptRef(*new XMLDocumentParser(fragment, WTFMove(prefixToNamespaceMap), defaultNamespaceURI, parserContentPolicy));
     }
 
     ~XMLDocumentParser();
@@ -89,7 +89,7 @@
 
 private:
     explicit XMLDocumentParser(Document&, FrameView* = nullptr);
-    XMLDocumentParser(DocumentFragment&, Element*, ParserContentPolicy);
+    XMLDocumentParser(DocumentFragment&, HashMap<AtomString, AtomString>&&, const AtomString&, ParserContentPolicy);
 
     void insert(SegmentedString&&) final;
     void append(RefPtr<StringImpl>&&) final;
@@ -180,9 +180,10 @@
     TextPosition m_scriptStartPosition;
 
     bool m_parsingFragment { false };
+
+    HashMap<AtomString, AtomString> m_prefixToNamespaceMap;
     AtomString m_defaultNamespaceURI;
 
-    HashMap<AtomString, AtomString> m_prefixToNamespaceMap;
     SegmentedString m_pendingSrc;
 };
 

Modified: trunk/Source/WebCore/xml/parser/XMLDocumentParserLibxml2.cpp (252229 => 252230)


--- trunk/Source/WebCore/xml/parser/XMLDocumentParserLibxml2.cpp	2019-11-08 02:46:09 UTC (rev 252229)
+++ trunk/Source/WebCore/xml/parser/XMLDocumentParserLibxml2.cpp	2019-11-08 02:52:40 UTC (rev 252230)
@@ -571,44 +571,16 @@
 {
 }
 
-XMLDocumentParser::XMLDocumentParser(DocumentFragment& fragment, Element* parentElement, ParserContentPolicy parserContentPolicy)
+XMLDocumentParser::XMLDocumentParser(DocumentFragment& fragment, HashMap<AtomString, AtomString>&& prefixToNamespaceMap, const AtomString& defaultNamespaceURI, ParserContentPolicy parserContentPolicy)
     : ScriptableDocumentParser(fragment.document(), parserContentPolicy)
     , m_pendingCallbacks(makeUnique<PendingCallbacks>())
     , m_currentNode(&fragment)
     , m_scriptStartPosition(TextPosition::belowRangePosition())
     , m_parsingFragment(true)
+    , m_prefixToNamespaceMap(WTFMove(prefixToNamespaceMap))
+    , m_defaultNamespaceURI(defaultNamespaceURI)
 {
     fragment.ref();
-
-    // Add namespaces based on the parent node
-    Vector<Element*> elemStack;
-    while (parentElement) {
-        elemStack.append(parentElement);
-
-        ContainerNode* node = parentElement->parentNode();
-        if (!is<Element>(node))
-            break;
-        parentElement = downcast<Element>(node);
-    }
-
-    if (elemStack.isEmpty())
-        return;
-
-    // FIXME: Share code with isDefaultNamespace() per http://www.whatwg.org/specs/web-apps/current-work/multipage/the-xhtml-syntax.html#parsing-xhtml-fragments
-    for (; !elemStack.isEmpty(); elemStack.removeLast()) {
-        Element* element = elemStack.last();
-        if (element->hasAttributes()) {
-            for (const Attribute& attribute : element->attributesIterator()) {
-                if (attribute.localName() == xmlnsAtom())
-                    m_defaultNamespaceURI = attribute.value();
-                else if (attribute.prefix() == xmlnsAtom())
-                    m_prefixToNamespaceMap.set(attribute.localName(), attribute.value());
-            }
-        }
-    }
-
-    if (m_defaultNamespaceURI.isNull())
-        m_defaultNamespaceURI = parentElement->namespaceURI();
 }
 
 XMLParserContext::~XMLParserContext()
@@ -769,6 +741,8 @@
     if (m_parsingFragment && uri.isNull()) {
         if (!prefix.isNull())
             uri = m_prefixToNamespaceMap.get(prefix);
+        else if (is<SVGElement>(m_currentNode) || localName == SVGNames::svgTag->localName())
+            uri = SVGNames::svgNamespaceURI;
         else
             uri = m_defaultNamespaceURI;
     }
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to