Default NamepaceURI must be gotten from the topmost parent before the SVG <foreignObject>
authorcommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 8 Nov 2019 02:52:40 +0000 (02:52 +0000)
committercommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 8 Nov 2019 02:52:40 +0000 (02:52 +0000)
https://bugs.webkit.org/show_bug.cgi?id=203868

Patch by Said Abou-Hallawa <sabouhallawa@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.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@252230 268f45cc-cd09-0410-ab3c-d52691b4dbfc

LayoutTests/ChangeLog
LayoutTests/svg/foreignObject/foreign-object-dynamic-parsing-expected.svg [new file with mode: 0644]
LayoutTests/svg/foreignObject/foreign-object-dynamic-parsing.svg [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/html/HTMLTableCellElement.cpp
Source/WebCore/xml/parser/XMLDocumentParser.cpp
Source/WebCore/xml/parser/XMLDocumentParser.h
Source/WebCore/xml/parser/XMLDocumentParserLibxml2.cpp

index e595ee7..9251983 100644 (file)
@@ -1,3 +1,13 @@
+2019-11-07  Said Abou-Hallawa  <sabouhallawa@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  <cdumez@apple.com>
 
         TestController may reuse a view that used window.open(), which prevents process-swapping and causes flakiness
diff --git a/LayoutTests/svg/foreignObject/foreign-object-dynamic-parsing-expected.svg b/LayoutTests/svg/foreignObject/foreign-object-dynamic-parsing-expected.svg
new file mode 100644 (file)
index 0000000..ad6c878
--- /dev/null
@@ -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>
diff --git a/LayoutTests/svg/foreignObject/foreign-object-dynamic-parsing.svg b/LayoutTests/svg/foreignObject/foreign-object-dynamic-parsing.svg
new file mode 100644 (file)
index 0000000..c053c28
--- /dev/null
@@ -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>
index 24d9cdf..1bd43db 100644 (file)
@@ -1,3 +1,36 @@
+2019-11-07  Said Abou-Hallawa  <sabouhallawa@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  <cdumez@apple.com>
 
         Use ActiveDOMObject::queueTaskKeepingObjectAlive() in DOMCache
index 52a9b4f..fbf8f73 100644 (file)
@@ -57,7 +57,7 @@ Ref<HTMLTableCellElement> HTMLTableCellElement::create(const QualifiedName& tagN
 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
index 7048892..77d94af 100644 (file)
@@ -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 @@ bool XMLDocumentParser::parseDocumentFragment(const String& chunk, DocumentFragm
         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.
index b3d9d93..d698563 100644 (file)
@@ -67,9 +67,9 @@ public:
     {
         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 @@ public:
 
 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 @@ private:
     TextPosition m_scriptStartPosition;
 
     bool m_parsingFragment { false };
-    AtomString m_defaultNamespaceURI;
 
     HashMap<AtomString, AtomString> m_prefixToNamespaceMap;
+    AtomString m_defaultNamespaceURI;
+
     SegmentedString m_pendingSrc;
 };
 
index b99b308..7f86c90 100644 (file)
@@ -571,44 +571,16 @@ XMLDocumentParser::XMLDocumentParser(Document& document, FrameView* frameView)
 {
 }
 
-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 @@ void XMLDocumentParser::startElementNs(const xmlChar* xmlLocalName, const xmlCha
     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;
     }