Namespace prefix is blindly followed when serializing
authorrwlbuis@webkit.org <rwlbuis@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 28 Aug 2013 22:25:19 +0000 (22:25 +0000)
committerrwlbuis@webkit.org <rwlbuis@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 28 Aug 2013 22:25:19 +0000 (22:25 +0000)
https://bugs.webkit.org/show_bug.cgi?id=19121
Serializer doesn't handling inconsistent prefixes properly
https://bugs.webkit.org/show_bug.cgi?id=117764
Attribute namespaces are serialized as if they were element ones
https://bugs.webkit.org/show_bug.cgi?id=22958

Reviewed by Ryosuke Niwa.

Source/WebCore:

Add code to make sure unique prefixes and namespace declarations are generated.
Unique prefix generation happens when:
- the same prefix is used to map to different namespaces or
- no prefix is given but the attribute is in a namespace.

This is done in order to not violate constraints listed in http://www.w3.org/TR/xml-names11/. In general
the pseudo code listed in http://www.w3.org/TR/DOM-Level-3-Core/namespaces-algorithms.html#normalizeDocumentAlgo
is used, doing the following for attributes:
if the attribute has a namespace then
  if the attribute has no prefix OR prefix is not declared OR conflicts with existing prefix mapping to different NS then
    try to find the matching in-scope declaration by looking up the prefix in the namespace -> prefix mapping, if found use that prefix
    else if the attribute prefix is not null AND not mapped in-scope, declare the prefix
    else generate a unique prefix for the namespace

To keep track of in-scope namespaces a prefix to namespace mapping is used.

Tests: fast/dom/XMLSerializer-attribute-namespace-prefix-conflicts.html
       fast/dom/XMLSerializer-same-prefix-different-namespaces-conflict.html
       fast/dom/XMLSerializer-setAttributeNS-namespace-no-prefix.html
       svg/custom/xlink-prefix-generation-in-attributes.html

* editing/MarkupAccumulator.cpp:
(WebCore::MarkupAccumulator::MarkupAccumulator):
(WebCore::MarkupAccumulator::shouldAddNamespaceAttribute):
(WebCore::MarkupAccumulator::appendNamespace):
(WebCore::MarkupAccumulator::generateUniquePrefix):
(WebCore::MarkupAccumulator::appendAttribute):
* editing/MarkupAccumulator.h:

LayoutTests:

Add tests to make sure unique prefixes and namespace declarations are generated for the
case when the same prefix is used to map to different namespaces. All testcases are based
on the testcases attached to the bugs.

* fast/dom/XMLSerializer-attribute-namespace-prefix-conflicts-expected.txt: Added.
* fast/dom/XMLSerializer-attribute-namespace-prefix-conflicts.html: Added.
* fast/dom/XMLSerializer-same-prefix-different-namespaces-conflict-expected.txt: Added.
* fast/dom/XMLSerializer-same-prefix-different-namespaces-conflict.html: Added.
* fast/dom/XMLSerializer-setAttributeNS-namespace-no-prefix-expected.txt: Added.
* fast/dom/XMLSerializer-setAttributeNS-namespace-no-prefix.html: Added.
* svg/custom/xlink-prefix-generation-in-attributes-expected.txt: Added.
* svg/custom/xlink-prefix-generation-in-attributes.html: Added.

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

12 files changed:
LayoutTests/ChangeLog
LayoutTests/fast/dom/XMLSerializer-attribute-namespace-prefix-conflicts-expected.txt [new file with mode: 0644]
LayoutTests/fast/dom/XMLSerializer-attribute-namespace-prefix-conflicts.html [new file with mode: 0644]
LayoutTests/fast/dom/XMLSerializer-same-prefix-different-namespaces-conflict-expected.txt [new file with mode: 0644]
LayoutTests/fast/dom/XMLSerializer-same-prefix-different-namespaces-conflict.html [new file with mode: 0644]
LayoutTests/fast/dom/XMLSerializer-setAttributeNS-namespace-no-prefix-expected.txt [new file with mode: 0644]
LayoutTests/fast/dom/XMLSerializer-setAttributeNS-namespace-no-prefix.html [new file with mode: 0644]
LayoutTests/svg/custom/xlink-prefix-generation-in-attributes-expected.txt [new file with mode: 0644]
LayoutTests/svg/custom/xlink-prefix-generation-in-attributes.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/editing/MarkupAccumulator.cpp
Source/WebCore/editing/MarkupAccumulator.h

index 63e820a..d9d281f 100644 (file)
@@ -1,3 +1,27 @@
+2013-08-28  Rob Buis  <rwlbuis@webkit.org>
+
+        Namespace prefix is blindly followed when serializing
+        https://bugs.webkit.org/show_bug.cgi?id=19121
+        Serializer doesn't handling inconsistent prefixes properly
+        https://bugs.webkit.org/show_bug.cgi?id=117764
+        Attribute namespaces are serialized as if they were element ones
+        https://bugs.webkit.org/show_bug.cgi?id=22958
+
+        Reviewed by Ryosuke Niwa.
+
+        Add tests to make sure unique prefixes and namespace declarations are generated for the
+        case when the same prefix is used to map to different namespaces. All testcases are based
+        on the testcases attached to the bugs.
+
+        * fast/dom/XMLSerializer-attribute-namespace-prefix-conflicts-expected.txt: Added.
+        * fast/dom/XMLSerializer-attribute-namespace-prefix-conflicts.html: Added.
+        * fast/dom/XMLSerializer-same-prefix-different-namespaces-conflict-expected.txt: Added.
+        * fast/dom/XMLSerializer-same-prefix-different-namespaces-conflict.html: Added.
+        * fast/dom/XMLSerializer-setAttributeNS-namespace-no-prefix-expected.txt: Added.
+        * fast/dom/XMLSerializer-setAttributeNS-namespace-no-prefix.html: Added.
+        * svg/custom/xlink-prefix-generation-in-attributes-expected.txt: Added.
+        * svg/custom/xlink-prefix-generation-in-attributes.html: Added.
+
 2013-08-28  Sam White  <samuel_white@apple.com>
 
         AX: Cancel button in search field not accessible.
diff --git a/LayoutTests/fast/dom/XMLSerializer-attribute-namespace-prefix-conflicts-expected.txt b/LayoutTests/fast/dom/XMLSerializer-attribute-namespace-prefix-conflicts-expected.txt
new file mode 100644 (file)
index 0000000..c7caed5
--- /dev/null
@@ -0,0 +1 @@
+<doc xmlns="http://www.example.com/ns1" xmlns:p="http://www.example.com/p" xmlns:NS2="http://www.example.com/ns2"><test xmlns="http://www.example.com/ns2" NS1:a="I am q" xmlns:NS1="http://www.example.com/q" p:b="I am p" xmlns:xlink="http://www.example.com/xlink" NS3:href="#foo" xmlns:NS3="http://www.w3.org/1999/xlink"/></doc>
diff --git a/LayoutTests/fast/dom/XMLSerializer-attribute-namespace-prefix-conflicts.html b/LayoutTests/fast/dom/XMLSerializer-attribute-namespace-prefix-conflicts.html
new file mode 100644 (file)
index 0000000..2352860
--- /dev/null
@@ -0,0 +1,35 @@
+<!DOCTYPE html>
+<html xmlns="http://www.w3.org/1999/xhtml">
+<head>
+    <script type="text/javascript">
+    function runTest()
+    {
+        if (window.testRunner)
+            testRunner.dumpAsText();
+
+        var xmlns = "http://www.w3.org/XML/1998/namespace";
+
+        var serializer = new XMLSerializer();
+
+        var doc = document.implementation.createDocument("http://www.example.com/ns1","doc",null);
+        doc.documentElement.setAttributeNS("http://www.w3.org/2000/xmlns/","xmlns:p","http://www.example.com/p");
+        doc.documentElement.setAttributeNS("http://www.w3.org/2000/xmlns/","xmlns:NS2","http://www.example.com/ns2");
+        var test = doc.createElementNS("http://www.example.com/ns2","test");
+        doc.documentElement.appendChild(test);
+        test.setAttributeNS("http://www.example.com/q","p:a","I am q");
+        test.setAttributeNS("http://www.example.com/p","p:b","I am p");
+        test.setAttributeNS("http://www.w3.org/2000/xmlns/","xmlns:xlink","http://www.example.com/xlink");
+        test.setAttributeNS("http://www.w3.org/1999/xlink","xlink:href","#foo");
+
+        var xmlString = serializer.serializeToString(doc);
+
+        var outputText = document.getElementById("output");
+        outputText.textContent = xmlString;
+    }
+    </script>
+</head>
+    <body onload="runTest()">
+        <div id="target"/>
+        <div id="output"/>
+    </body>
+</html>
diff --git a/LayoutTests/fast/dom/XMLSerializer-same-prefix-different-namespaces-conflict-expected.txt b/LayoutTests/fast/dom/XMLSerializer-same-prefix-different-namespaces-conflict-expected.txt
new file mode 100644 (file)
index 0000000..f6c69da
--- /dev/null
@@ -0,0 +1 @@
+<x:y xmlns:x="ns1" NS1:z="val" xmlns:NS1="ns2" NS2:w="val" xmlns:NS2="ns3"/>
diff --git a/LayoutTests/fast/dom/XMLSerializer-same-prefix-different-namespaces-conflict.html b/LayoutTests/fast/dom/XMLSerializer-same-prefix-different-namespaces-conflict.html
new file mode 100644 (file)
index 0000000..7bbae44
--- /dev/null
@@ -0,0 +1,28 @@
+<!DOCTYPE html>
+<html>
+<head>
+    <script type="text/javascript">
+    function runTest()
+    {
+        if (window.testRunner)
+            testRunner.dumpAsText();
+
+        var doc = document.implementation.createDocument("", "", null);
+        var parent = doc.firstChild;
+        var element = doc.createElementNS("ns1", "x:y");
+        element.setAttributeNS("ns2", "x:z", "val");
+        element.setAttributeNS("ns3", "w", "val");
+        doc.appendChild(element);
+        var serializer = new XMLSerializer();
+        var xmlString = serializer.serializeToString(doc);
+
+        var outputText = document.getElementById("output");
+        outputText.textContent = xmlString;
+    }
+    </script>
+</head>
+    <body onload="runTest()">
+        <div id="target"/>
+        <div id="output"/>
+    </body>
+</html>
diff --git a/LayoutTests/fast/dom/XMLSerializer-setAttributeNS-namespace-no-prefix-expected.txt b/LayoutTests/fast/dom/XMLSerializer-setAttributeNS-namespace-no-prefix-expected.txt
new file mode 100644 (file)
index 0000000..7ef22e9
--- /dev/null
@@ -0,0 +1 @@
+PASS
diff --git a/LayoutTests/fast/dom/XMLSerializer-setAttributeNS-namespace-no-prefix.html b/LayoutTests/fast/dom/XMLSerializer-setAttributeNS-namespace-no-prefix.html
new file mode 100644 (file)
index 0000000..c240807
--- /dev/null
@@ -0,0 +1,24 @@
+<!DOCTYPE html>
+<html xmlns="http://www.w3.org/1999/xhtml">
+<head>
+    <script type="text/javascript">
+    function runTest()
+    {
+        if (window.testRunner)
+            testRunner.dumpAsText();
+
+        var outputText = document.getElementById("output");
+
+        document.getElementById('target').setAttributeNS('ns', 'title', 'value');
+        if (document.getElementById('target').outerHTML.match('xmlns="ns"'))
+          outputText.textContent = 'FAIL: ' + document.getElementById('target').outerHTML;
+        else
+          outputText.textContent = "PASS";
+    }
+    </script>
+</head>
+    <body onload="runTest()">
+        <div id="target"/>
+        <div id="output"/>
+    </body>
+</html>
diff --git a/LayoutTests/svg/custom/xlink-prefix-generation-in-attributes-expected.txt b/LayoutTests/svg/custom/xlink-prefix-generation-in-attributes-expected.txt
new file mode 100644 (file)
index 0000000..ec257a5
--- /dev/null
@@ -0,0 +1,4 @@
+<svg xmlns="http://www.w3.org/2000/svg" NS1:href="#foo" xmlns:NS1="http://www.w3.org/1999/xlink"/>
+<svg xmlns="http://www.w3.org/2000/svg" NS1:href="#foo" xmlns:NS1="http://www.w3.org/1999/xlink"/>
+<svg xmlns="http://www.w3.org/2000/svg" xlink:href="#foo" xmlns:xlink="http://www.w3.org/1999/xlink"/>
+<svg xmlns="http://www.w3.org/2000/svg" xlink:href="#foo" xmlns:xlink="http://www.w3.org/1999/xlink"/>
diff --git a/LayoutTests/svg/custom/xlink-prefix-generation-in-attributes.html b/LayoutTests/svg/custom/xlink-prefix-generation-in-attributes.html
new file mode 100644 (file)
index 0000000..8775576
--- /dev/null
@@ -0,0 +1,49 @@
+<!DOCTYPE html>
+<html xmlns="http://www.w3.org/1999/xhtml">
+<head>
+    <script type="text/javascript">
+    function runTest()
+    {
+        if (window.testRunner)
+            testRunner.dumpAsText();
+
+        var svgns = "http://www.w3.org/2000/svg";
+        var xlinkns = "http://www.w3.org/1999/xlink";
+
+        var serializer = new XMLSerializer();
+
+        // Example 1: Create a document with a namespaced attribute but via the DOM API but do not specific a prefix.
+        var doc = document.implementation.createDocument(svgns, "svg", null);
+        doc.documentElement.setAttributeNS(xlinkns, "href", "#foo");
+        var xml = serializer.serializeToString(doc);
+        document.getElementById("svgoutput").textContent = xml + "\n";
+   
+        // Example 2: Attempt to fix the document by setting the attribute.  The Node.prefix property should now contain "xlink".
+        doc = document.implementation.createDocument(svgns, "svg", null);
+        doc.documentElement.setAttributeNS(xlinkns, "href", "#foo");
+        doc.documentElement.setAttributeNS(xlinkns, "xlink:href", "#foo");
+        xml = serializer.serializeToString(doc);
+        document.getElementById("svgoutput2").textContent = xml + "\n";
+   
+        // Example 3: Attempt to fix the document by setting Node.prefix.  The Node.prefix property should now contain "xlink".
+        doc = document.implementation.createDocument(svgns, "svg", null);
+        doc.documentElement.setAttributeNS(xlinkns, "href", "#foo");
+        doc.documentElement.attributes[0].prefix = "xlink";
+        xml = serializer.serializeToString(doc);
+        document.getElementById("svgoutput3").textContent = xml + "\n";
+   
+        // Example 4: Create the document with prefixes specified.  The Node.prefix property should now contain "xlink".
+        doc = document.implementation.createDocument(svgns, "svg", null);
+        doc.documentElement.setAttributeNS(xlinkns, "xlink:href", "#foo");
+        xml = serializer.serializeToString(doc);
+        document.getElementById("svgoutput4").textContent = xml + "\n";
+    }
+    </script>
+</head>
+    <body onload="runTest()">
+        <div id="svgoutput"></div>
+        <div id="svgoutput2"></div>
+        <div id="svgoutput3"></div>
+        <div id="svgoutput4"></div>
+    </body>
+</html>
index e9955ba..ed04dce 100644 (file)
@@ -1,3 +1,43 @@
+2013-08-28  Rob Buis  <rwlbuis@webkit.org>
+
+        Namespace prefix is blindly followed when serializing
+        https://bugs.webkit.org/show_bug.cgi?id=19121
+        Serializer doesn't handling inconsistent prefixes properly
+        https://bugs.webkit.org/show_bug.cgi?id=117764
+        Attribute namespaces are serialized as if they were element ones
+        https://bugs.webkit.org/show_bug.cgi?id=22958
+
+        Reviewed by Ryosuke Niwa.
+
+        Add code to make sure unique prefixes and namespace declarations are generated.
+        Unique prefix generation happens when:
+        - the same prefix is used to map to different namespaces or
+        - no prefix is given but the attribute is in a namespace.
+
+        This is done in order to not violate constraints listed in http://www.w3.org/TR/xml-names11/. In general
+        the pseudo code listed in http://www.w3.org/TR/DOM-Level-3-Core/namespaces-algorithms.html#normalizeDocumentAlgo
+        is used, doing the following for attributes:
+        if the attribute has a namespace then
+          if the attribute has no prefix OR prefix is not declared OR conflicts with existing prefix mapping to different NS then
+            try to find the matching in-scope declaration by looking up the prefix in the namespace -> prefix mapping, if found use that prefix
+            else if the attribute prefix is not null AND not mapped in-scope, declare the prefix
+            else generate a unique prefix for the namespace
+
+        To keep track of in-scope namespaces a prefix to namespace mapping is used. 
+
+        Tests: fast/dom/XMLSerializer-attribute-namespace-prefix-conflicts.html
+               fast/dom/XMLSerializer-same-prefix-different-namespaces-conflict.html
+               fast/dom/XMLSerializer-setAttributeNS-namespace-no-prefix.html
+               svg/custom/xlink-prefix-generation-in-attributes.html
+
+        * editing/MarkupAccumulator.cpp:
+        (WebCore::MarkupAccumulator::MarkupAccumulator):
+        (WebCore::MarkupAccumulator::shouldAddNamespaceAttribute):
+        (WebCore::MarkupAccumulator::appendNamespace):
+        (WebCore::MarkupAccumulator::generateUniquePrefix):
+        (WebCore::MarkupAccumulator::appendAttribute):
+        * editing/MarkupAccumulator.h:
+
 2013-08-28  Sam White  <samuel_white@apple.com>
 
         AX: Cancel button in search field not accessible.
index 51c1cad..7477076 100644 (file)
@@ -105,6 +105,7 @@ MarkupAccumulator::MarkupAccumulator(Vector<Node*>* nodes, EAbsoluteURLs resolve
     , m_range(range)
     , m_resolveURLsMethod(resolveUrlsMethod)
     , m_fragmentSerialization(fragmentSerialization)
+    , m_prefixLevel(0)
 {
 }
 
@@ -287,6 +288,7 @@ bool MarkupAccumulator::shouldAddNamespaceAttribute(const Attribute& attribute,
     QualifiedName xmlnsPrefixAttr(xmlnsAtom, attribute.localName(), XMLNSNames::xmlnsNamespaceURI);
     if (attribute.name() == xmlnsPrefixAttr) {
         namespaces.set(attribute.localName().impl(), attribute.value().impl());
+        namespaces.set(attribute.value().impl(), attribute.localName().impl());
         return false;
     }
 
@@ -309,8 +311,14 @@ void MarkupAccumulator::appendNamespace(StringBuilder& result, const AtomicStrin
     // Use emptyAtoms's impl() for both null and empty strings since the HashMap can't handle 0 as a key
     AtomicStringImpl* pre = prefix.isEmpty() ? emptyAtom.impl() : prefix.impl();
     AtomicStringImpl* foundNS = namespaces.get(pre);
-    if (foundNS != namespaceURI.impl() && !namespaces.get(namespaceURI.impl())) {
+    if (foundNS != namespaceURI.impl()) {
         namespaces.set(pre, namespaceURI.impl());
+        // Add namespace to prefix pair so we can do constraint checking later.
+        if (inXMLFragmentSerialization() && !prefix.isEmpty())
+            namespaces.set(namespaceURI.impl(), pre);
+        // Make sure xml prefix and namespace are always known to uphold the constraints listed at http://www.w3.org/TR/xml-names11/#xmlReserved.
+        if (namespaceURI.impl() == XMLNames::xmlNamespaceURI.impl())
+            return;
         result.append(' ');
         result.append(xmlnsAtom.string());
         if (!prefix.isEmpty()) {
@@ -466,6 +474,24 @@ static inline bool attributeIsInSerializedNamespace(const Attribute& attribute)
         || attribute.namespaceURI() == XMLNSNames::xmlnsNamespaceURI;
 }
 
+void MarkupAccumulator::generateUniquePrefix(QualifiedName& prefixedName, const Namespaces& namespaces)
+{
+    // http://www.w3.org/TR/DOM-Level-3-Core/namespaces-algorithms.html#normalizeDocumentAlgo
+    // Find a prefix following the pattern "NS" + index (starting at 1) and make sure this
+    // prefix is not declared in the current scope.
+    StringBuilder builder;
+    do {
+        builder.clear();
+        builder.append("NS");
+        builder.appendNumber(++m_prefixLevel);
+        const AtomicString& name = builder.toAtomicString();
+        if (!namespaces.get(name.impl())) {
+            prefixedName.setPrefix(name);
+            return;
+        }
+    } while (true);
+}
+
 void MarkupAccumulator::appendAttribute(StringBuilder& result, Element* element, const Attribute& attribute, Namespaces* namespaces)
 {
     bool documentIsHTML = element->document()->isHTMLDocument();
@@ -476,15 +502,18 @@ void MarkupAccumulator::appendAttribute(StringBuilder& result, Element* element,
     if (documentIsHTML && !attributeIsInSerializedNamespace(attribute))
         result.append(attribute.name().localName());
     else {
-        if (attribute.namespaceURI() == XLinkNames::xlinkNamespaceURI) {
-            if (!attribute.prefix())
-                prefixedName.setPrefix(xlinkAtom);
-        } else if (attribute.namespaceURI() == XMLNames::xmlNamespaceURI) {
-            if (!attribute.prefix())
-                prefixedName.setPrefix(xmlAtom);
-        } else if (attribute.namespaceURI() == XMLNSNames::xmlnsNamespaceURI) {
-            if (attribute.name() != XMLNSNames::xmlnsAttr && !attribute.prefix())
-                prefixedName.setPrefix(xmlnsAtom);
+        if (!attribute.namespaceURI().isEmpty()) {
+            AtomicStringImpl* foundNS = namespaces && attribute.prefix().impl() ? namespaces->get(attribute.prefix().impl()) : 0;
+            bool prefixIsAlreadyMappedToOtherNS = foundNS && foundNS != attribute.namespaceURI().impl();
+            if (attribute.prefix().isEmpty() || !foundNS || prefixIsAlreadyMappedToOtherNS) {
+                if (AtomicStringImpl* prefix = namespaces ? namespaces->get(attribute.namespaceURI().impl()) : 0)
+                    prefixedName.setPrefix(AtomicString(prefix));
+                else {
+                    bool shouldBeDeclaredUsingAppendNamespace = !attribute.prefix().isEmpty() && !foundNS;
+                    if (!shouldBeDeclaredUsingAppendNamespace && attribute.localName() != xmlnsAtom && namespaces)
+                        generateUniquePrefix(prefixedName, *namespaces);
+                }
+            }
         }
         result.append(prefixedName.toString());
     }
index 9148a40..b872740 100644 (file)
@@ -112,10 +112,12 @@ private:
     void appendQuotedURLAttributeValue(StringBuilder&, const Element*, const Attribute&);
     void serializeNodesWithNamespaces(Node* targetNode, Node* nodeToSkip, EChildrenOnly, const Namespaces*, Vector<QualifiedName>* tagNamesToSkip);
     bool inXMLFragmentSerialization() const { return m_fragmentSerialization == XMLFragmentSerialization; }
+    void generateUniquePrefix(QualifiedName&, const Namespaces&);
 
     StringBuilder m_markup;
     const EAbsoluteURLs m_resolveURLsMethod;
     EFragmentSerialization m_fragmentSerialization;
+    unsigned m_prefixLevel;
 };
 
 }