Element.outerHTML is missing attribute prefixes in some cases in HTML documents
authorcdumez@apple.com <cdumez@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 31 Jul 2019 15:12:41 +0000 (15:12 +0000)
committercdumez@apple.com <cdumez@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 31 Jul 2019 15:12:41 +0000 (15:12 +0000)
https://bugs.webkit.org/show_bug.cgi?id=200283

Reviewed by Ryosuke Niwa.

Source/WebCore:

When HTML serializing a prefixed element attribute, we should always serialize the
prefix as per [1]. However, our code was only serializing the well-known ones (xml,
xmlns & xlink).

[1] https://html.spec.whatwg.org/#attribute's-serialised-name

Test: fast/dom/Element/outerHTML-prefixed-attribute.html

* editing/MarkupAccumulator.cpp:
(WebCore::htmlAttributeSerialization):
(WebCore::MarkupAccumulator::xmlAttributeSerialization):
(WebCore::MarkupAccumulator::appendAttribute):
* editing/MarkupAccumulator.h:

LayoutTests:

Add layout test coverage.

* fast/dom/Element/outerHTML-prefixed-attribute-expected.txt: Added.
* fast/dom/Element/outerHTML-prefixed-attribute.html: Added.

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

LayoutTests/ChangeLog
LayoutTests/fast/dom/Element/outerHTML-prefixed-attribute-expected.txt [new file with mode: 0644]
LayoutTests/fast/dom/Element/outerHTML-prefixed-attribute.html [new file with mode: 0644]
LayoutTests/imported/w3c/web-platform-tests/html/syntax/serializing-html-fragments/serializing-expected.txt
Source/WebCore/ChangeLog
Source/WebCore/editing/MarkupAccumulator.cpp
Source/WebCore/editing/MarkupAccumulator.h

index 84ca4b3..f2b51c7 100644 (file)
@@ -1,3 +1,15 @@
+2019-07-31  Chris Dumez  <cdumez@apple.com>
+
+        Element.outerHTML is missing attribute prefixes in some cases in HTML documents
+        https://bugs.webkit.org/show_bug.cgi?id=200283
+
+        Reviewed by Ryosuke Niwa.
+
+        Add layout test coverage.
+
+        * fast/dom/Element/outerHTML-prefixed-attribute-expected.txt: Added.
+        * fast/dom/Element/outerHTML-prefixed-attribute.html: Added.
+
 2019-07-31  Zan Dobersek  <zdobersek@igalia.com>
 
         Unreviewed WPE and GTK gardening. Adding some failure expectations as
diff --git a/LayoutTests/fast/dom/Element/outerHTML-prefixed-attribute-expected.txt b/LayoutTests/fast/dom/Element/outerHTML-prefixed-attribute-expected.txt
new file mode 100644 (file)
index 0000000..85b8eff
--- /dev/null
@@ -0,0 +1,10 @@
+Makes sure prefixes are properly serialized by Element.outerHTML in an HTML document.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS g.outerHTML is "<g ex:attr=\"1\"></g>"
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
diff --git a/LayoutTests/fast/dom/Element/outerHTML-prefixed-attribute.html b/LayoutTests/fast/dom/Element/outerHTML-prefixed-attribute.html
new file mode 100644 (file)
index 0000000..19ff5c9
--- /dev/null
@@ -0,0 +1,16 @@
+<!DOCTYPE html>
+<html>
+<body>
+<script src="../../../resources/js-test.js"></script>
+<svg xmlns="http://www.w3.org/2000/svg">
+  <g></g>
+</svg>
+<script>
+description("Makes sure prefixes are properly serialized by Element.outerHTML in an HTML document.");
+
+let g = document.querySelector("g")
+g.setAttributeNS("http://example.com","ex:attr","1")
+shouldBeEqualToString("g.outerHTML", "<g ex:attr=\"1\"></g>");
+</script>
+</body>
+</html>
index fb783fd..1feb915 100644 (file)
@@ -59,13 +59,13 @@ PASS innerHTML Attribute in the XML namespace
 PASS innerHTML Attribute in the XML namespace with the prefix not set to xml: 
 PASS innerHTML Non-'xmlns' attribute in the xmlns namespace 
 PASS innerHTML 'xmlns' attribute in the xmlns namespace 
-FAIL innerHTML Attribute in non-standard namespace assert_equals: expected "<svg abc:def=\"test\"></svg>" but got "<svg def=\"test\"></svg>"
+PASS innerHTML Attribute in non-standard namespace 
 PASS innerHTML <span> starting with U+000A 
 PASS outerHTML Attribute in the XML namespace 
 PASS outerHTML Attribute in the XML namespace with the prefix not set to xml: 
 PASS outerHTML Non-'xmlns' attribute in the xmlns namespace 
 PASS outerHTML 'xmlns' attribute in the xmlns namespace 
-FAIL outerHTML Attribute in non-standard namespace assert_equals: expected "<span><svg abc:def=\"test\"></svg></span>" but got "<span><svg def=\"test\"></svg></span>"
+PASS outerHTML Attribute in non-standard namespace 
 PASS outerHTML <span> starting with U+000A 
 PASS innerHTML <pre> context starting with U+000A 
 PASS innerHTML <textarea> context starting with U+000A 
index add1c4e..10e71c5 100644 (file)
@@ -1,3 +1,24 @@
+2019-07-31  Chris Dumez  <cdumez@apple.com>
+
+        Element.outerHTML is missing attribute prefixes in some cases in HTML documents
+        https://bugs.webkit.org/show_bug.cgi?id=200283
+
+        Reviewed by Ryosuke Niwa.
+
+        When HTML serializing a prefixed element attribute, we should always serialize the
+        prefix as per [1]. However, our code was only serializing the well-known ones (xml,
+        xmlns & xlink).
+
+        [1] https://html.spec.whatwg.org/#attribute's-serialised-name
+
+        Test: fast/dom/Element/outerHTML-prefixed-attribute.html
+
+        * editing/MarkupAccumulator.cpp:
+        (WebCore::htmlAttributeSerialization):
+        (WebCore::MarkupAccumulator::xmlAttributeSerialization):
+        (WebCore::MarkupAccumulator::appendAttribute):
+        * editing/MarkupAccumulator.h:
+
 2019-07-31  Loïc Yhuel  <loic.yhuel@softathome.com>
 
         [GStreamer] Fix printf format warnings for 32-bit build in GST traces
index 7e9b47f..a9bb04c 100644 (file)
@@ -514,13 +514,6 @@ void MarkupAccumulator::appendCloseTag(StringBuilder& result, const Element& ele
     result.append('>');
 }
 
-static inline bool attributeIsInSerializedNamespace(const Attribute& attribute)
-{
-    return attribute.namespaceURI() == XMLNames::xmlNamespaceURI
-        || attribute.namespaceURI() == XLinkNames::xlinkNamespaceURI
-        || 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
@@ -539,35 +532,61 @@ void MarkupAccumulator::generateUniquePrefix(QualifiedName& prefixedName, const
     } while (true);
 }
 
-void MarkupAccumulator::appendAttribute(StringBuilder& result, const Element& element, const Attribute& attribute, Namespaces* namespaces)
+// https://html.spec.whatwg.org/#attribute's-serialised-name
+static String htmlAttributeSerialization(const Attribute& attribute)
 {
-    bool isSerializingHTML = element.document().isHTMLDocument() && !inXMLFragmentSerialization();
+    if (attribute.namespaceURI().isEmpty())
+        return attribute.name().localName();
 
-    result.append(' ');
+    QualifiedName prefixedName = attribute.name();
+    if (attribute.namespaceURI() == XMLNames::xmlNamespaceURI)
+        prefixedName.setPrefix(xmlAtom());
+    else if (attribute.namespaceURI() == XMLNSNames::xmlnsNamespaceURI) {
+        if (prefixedName.localName() == xmlnsAtom())
+            return xmlnsAtom();
+        prefixedName.setPrefix(xmlnsAtom());
+    } else if (attribute.namespaceURI() == XLinkNames::xlinkNamespaceURI)
+        prefixedName.setPrefix(AtomString("xlink", AtomString::ConstructFromLiteral));
+    return prefixedName.toString();
+}
 
+// https://w3c.github.io/DOM-Parsing/#dfn-xml-serialization-of-the-attributes
+QualifiedName MarkupAccumulator::xmlAttributeSerialization(const Attribute& attribute, Namespaces* namespaces)
+{
     QualifiedName prefixedName = attribute.name();
-    if (isSerializingHTML && !attributeIsInSerializedNamespace(attribute))
-        result.append(attribute.name().localName());
-    else {
-        if (!attribute.namespaceURI().isEmpty()) {
-            if (attribute.namespaceURI() == XMLNames::xmlNamespaceURI) {
-                // Always use xml as prefix if the namespace is the XML namespace.
-                prefixedName.setPrefix(xmlAtom());
-            } else {
-                AtomStringImpl* 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 (AtomStringImpl* prefix = namespaces ? namespaces->get(attribute.namespaceURI().impl()) : 0)
-                        prefixedName.setPrefix(AtomString(prefix));
-                    else {
-                        bool shouldBeDeclaredUsingAppendNamespace = !attribute.prefix().isEmpty() && !foundNS;
-                        if (!shouldBeDeclaredUsingAppendNamespace && attribute.localName() != xmlnsAtom() && namespaces)
-                            generateUniquePrefix(prefixedName, *namespaces);
-                    }
+    if (!attribute.namespaceURI().isEmpty()) {
+        if (attribute.namespaceURI() == XMLNames::xmlNamespaceURI) {
+            // Always use xml as prefix if the namespace is the XML namespace.
+            prefixedName.setPrefix(xmlAtom());
+        } else {
+            AtomStringImpl* foundNS = namespaces && attribute.prefix().impl() ? namespaces->get(attribute.prefix().impl()) : nullptr;
+            bool prefixIsAlreadyMappedToOtherNS = foundNS && foundNS != attribute.namespaceURI().impl();
+            if (attribute.prefix().isEmpty() || !foundNS || prefixIsAlreadyMappedToOtherNS) {
+                if (AtomStringImpl* prefix = namespaces ? namespaces->get(attribute.namespaceURI().impl()) : nullptr)
+                    prefixedName.setPrefix(AtomString(prefix));
+                else {
+                    bool shouldBeDeclaredUsingAppendNamespace = !attribute.prefix().isEmpty() && !foundNS;
+                    if (!shouldBeDeclaredUsingAppendNamespace && attribute.localName() != xmlnsAtom() && namespaces)
+                        generateUniquePrefix(prefixedName, *namespaces);
                 }
             }
         }
-        result.append(prefixedName.toString());
+    }
+    return prefixedName;
+}
+
+void MarkupAccumulator::appendAttribute(StringBuilder& result, const Element& element, const Attribute& attribute, Namespaces* namespaces)
+{
+    bool isSerializingHTML = element.document().isHTMLDocument() && !inXMLFragmentSerialization();
+
+    result.append(' ');
+
+    Optional<QualifiedName> effectiveXMLPrefixedName;
+    if (isSerializingHTML)
+        result.append(htmlAttributeSerialization(attribute));
+    else {
+        effectiveXMLPrefixedName = xmlAttributeSerialization(attribute, namespaces);
+        result.append(effectiveXMLPrefixedName->toString());
     }
 
     result.append('=');
@@ -581,7 +600,7 @@ void MarkupAccumulator::appendAttribute(StringBuilder& result, const Element& el
     }
 
     if (!isSerializingHTML && namespaces && shouldAddNamespaceAttribute(attribute, *namespaces))
-        appendNamespace(result, prefixedName.prefix(), prefixedName.namespaceURI(), *namespaces);
+        appendNamespace(result, effectiveXMLPrefixedName->prefix(), effectiveXMLPrefixedName->namespaceURI(), *namespaces);
 }
 
 void MarkupAccumulator::appendCDATASection(StringBuilder& result, const String& section)
index 7c7bbf3..5487606 100644 (file)
@@ -113,6 +113,7 @@ private:
     void serializeNodesWithNamespaces(Node& targetNode, SerializedNodes, const Namespaces*, Vector<QualifiedName>* tagNamesToSkip);
     bool inXMLFragmentSerialization() const { return m_serializationSyntax == SerializationSyntax::XML; }
     void generateUniquePrefix(QualifiedName&, const Namespaces&);
+    QualifiedName xmlAttributeSerialization(const Attribute&, Namespaces*);
 
     StringBuilder m_markup;
     const ResolveURLs m_resolveURLs;