Rename MarkupAccumulator::appendStartTag, appendElement, etc... for clarity
authorrniwa@webkit.org <rniwa@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 11 Oct 2018 02:27:18 +0000 (02:27 +0000)
committerrniwa@webkit.org <rniwa@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 11 Oct 2018 02:27:18 +0000 (02:27 +0000)
https://bugs.webkit.org/show_bug.cgi?id=190308

Reviewed by Darin Adler.

Renamed appendStartTag and appendEndTag to startAppendingNode and endAppendingNode since serialize any node,
not just elements which produce start and end tags.

Renamed appendElement and appendEndElement to appendStartTag and appendEndTag since that's what they do.

* editing/MarkupAccumulator.cpp:
(WebCore::elementCannotHaveEndTag): Made this a static local function.
(WebCore::shouldSelfClose): Ditto.
(WebCore::MarkupAccumulator::serializeNodesWithNamespaces):
(WebCore::MarkupAccumulator::startAppendingNode): Renamed from appendStartTag.
(WebCore::MarkupAccumulator::endAppendingNode): Renamed from appendEndElement and merged appendEndMarkup here.
(WebCore::MarkupAccumulator::appendTextSubstring): Deleted. This was only used by StyledMarkupAccumulator.
(WebCore::MarkupAccumulator::appendStringView): Added.
(WebCore::MarkupAccumulator::appendStartTag): Renamed from appendElement.
(WebCore::MarkupAccumulator::appendCloseTag):
(WebCore::MarkupAccumulator::appendNonElementNode): Renamed from appendStartMarkup. No longer serializes
an element. StyledMarkupAccumulator had a check before calling this function already so this clarifies
the purpose of this function.
(WebCore::MarkupAccumulator::appendElement): Deleted.
(WebCore::MarkupAccumulator::appendEndMarkup): Deleted. Merged into appendEndTag.
* editing/MarkupAccumulator.h:
(WebCore::MarkupAccumulator::appendNodeEnd): Renamed from appendEndTag.
* editing/markup.cpp:
(WebCore::StyledMarkupAccumulator::wrapWithNode):
(WebCore::StyledMarkupAccumulator::appendStartTag): Renamed from appendElement.
(WebCore::StyledMarkupAccumulator::appendEndTag): Renamed from appendEndElement.
(WebCore::StyledMarkupAccumulator::traverseNodesForSerialization):
(WebCore::StyledMarkupAccumulator::appendNodeToPreserveMSOList): Use String::substring directly instead of
going through appendTextSubstring which has been deleted.
* page/PageSerializer.cpp:
(WebCore::PageSerializer::SerializerMarkupAccumulator::appendStartTag): Renamed from appendElement.
(WebCore::PageSerializer::SerializerMarkupAccumulator::appendEndTag): Renamed from appendEndElement.

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

Source/WebCore/ChangeLog
Source/WebCore/editing/MarkupAccumulator.cpp
Source/WebCore/editing/MarkupAccumulator.h
Source/WebCore/editing/markup.cpp
Source/WebCore/page/PageSerializer.cpp

index cc6c707..befbc32 100644 (file)
@@ -1,3 +1,43 @@
+2018-10-05  Ryosuke Niwa  <rniwa@webkit.org>
+
+        Rename MarkupAccumulator::appendStartTag, appendElement, etc... for clarity
+        https://bugs.webkit.org/show_bug.cgi?id=190308
+
+        Reviewed by Darin Adler.
+
+        Renamed appendStartTag and appendEndTag to startAppendingNode and endAppendingNode since serialize any node,
+        not just elements which produce start and end tags.
+
+        Renamed appendElement and appendEndElement to appendStartTag and appendEndTag since that's what they do.
+
+        * editing/MarkupAccumulator.cpp:
+        (WebCore::elementCannotHaveEndTag): Made this a static local function.
+        (WebCore::shouldSelfClose): Ditto.
+        (WebCore::MarkupAccumulator::serializeNodesWithNamespaces):
+        (WebCore::MarkupAccumulator::startAppendingNode): Renamed from appendStartTag.
+        (WebCore::MarkupAccumulator::endAppendingNode): Renamed from appendEndElement and merged appendEndMarkup here.
+        (WebCore::MarkupAccumulator::appendTextSubstring): Deleted. This was only used by StyledMarkupAccumulator.
+        (WebCore::MarkupAccumulator::appendStringView): Added.
+        (WebCore::MarkupAccumulator::appendStartTag): Renamed from appendElement.
+        (WebCore::MarkupAccumulator::appendCloseTag):
+        (WebCore::MarkupAccumulator::appendNonElementNode): Renamed from appendStartMarkup. No longer serializes
+        an element. StyledMarkupAccumulator had a check before calling this function already so this clarifies
+        the purpose of this function.
+        (WebCore::MarkupAccumulator::appendElement): Deleted.
+        (WebCore::MarkupAccumulator::appendEndMarkup): Deleted. Merged into appendEndTag.
+        * editing/MarkupAccumulator.h:
+        (WebCore::MarkupAccumulator::appendNodeEnd): Renamed from appendEndTag.
+        * editing/markup.cpp:
+        (WebCore::StyledMarkupAccumulator::wrapWithNode):
+        (WebCore::StyledMarkupAccumulator::appendStartTag): Renamed from appendElement.
+        (WebCore::StyledMarkupAccumulator::appendEndTag): Renamed from appendEndElement.
+        (WebCore::StyledMarkupAccumulator::traverseNodesForSerialization):
+        (WebCore::StyledMarkupAccumulator::appendNodeToPreserveMSOList): Use String::substring directly instead of
+        going through appendTextSubstring which has been deleted.
+        * page/PageSerializer.cpp:
+        (WebCore::PageSerializer::SerializerMarkupAccumulator::appendStartTag): Renamed from appendElement.
+        (WebCore::PageSerializer::SerializerMarkupAccumulator::appendEndTag): Renamed from appendEndElement.
+
 2018-10-10  Ryan Haddad  <ryanhaddad@apple.com>
 
         Unreviewed, attempt to fix the build with current SDKs.
index 45aee59..50efdc1 100644 (file)
@@ -85,6 +85,61 @@ static const uint8_t entityMap[maximumEscapedentityCharacter + 1] = {
     EntitySubstitutionNbspIndex // noBreakSpace.
 };
 
+static bool elementCannotHaveEndTag(const Node& node)
+{
+    if (!is<HTMLElement>(node))
+        return false;
+
+    // From https://html.spec.whatwg.org/#serialising-html-fragments:
+    // If current node is an area, base, basefont, bgsound, br, col, embed, frame, hr, img,
+    // input, keygen, link, meta, param, source, track or wbr element, then continue on to
+    // the next child node at this point.
+    static const AtomicStringImpl* localNames[] = {
+        areaTag->localName().impl(),
+        baseTag->localName().impl(),
+        basefontTag->localName().impl(),
+        bgsoundTag->localName().impl(),
+        brTag->localName().impl(),
+        colTag->localName().impl(),
+        embedTag->localName().impl(),
+        frameTag->localName().impl(),
+        hrTag->localName().impl(),
+        imgTag->localName().impl(),
+        inputTag->localName().impl(),
+        keygenTag->localName().impl(),
+        linkTag->localName().impl(),
+        metaTag->localName().impl(),
+        paramTag->localName().impl(),
+        sourceTag->localName().impl(),
+        trackTag->localName().impl(),
+        wbrTag->localName().impl()
+    };
+
+    auto* elementName = downcast<HTMLElement>(node).localName().impl();
+    for (auto* name : localNames) {
+        if (name == elementName)
+            return true;
+    }
+
+    return false;
+}
+
+// Rules of self-closure
+// 1. No elements in HTML documents use the self-closing syntax.
+// 2. Elements w/ children never self-close because they use a separate end tag.
+// 3. HTML elements which do not have a "forbidden" end tag will close with a separate end tag.
+// 4. Other elements self-close.
+static bool shouldSelfClose(const Element& element, SerializationSyntax syntax)
+{
+    if (syntax != SerializationSyntax::XML && element.document().isHTMLDocument())
+        return false;
+    if (element.hasChildNodes())
+        return false;
+    if (element.isHTMLElement() && !elementCannotHaveEndTag(element))
+        return false;
+    return true;
+}
+
 template<typename CharacterType>
 static inline void appendCharactersReplacingEntitiesInternal(StringBuilder& result, const String& source, unsigned offset, unsigned length, EntityMask entityMask)
 {
@@ -150,7 +205,7 @@ void MarkupAccumulator::serializeNodesWithNamespaces(Node& targetNode, Serialize
     }
 
     if (root == SerializedNodes::SubtreeIncludingNode)
-        appendStartTag(targetNode, &namespaceHash);
+        startAppendingNode(targetNode, &namespaceHash);
 
     if (targetNode.document().isHTMLDocument() && elementCannotHaveEndTag(targetNode))
         return;
@@ -160,7 +215,7 @@ void MarkupAccumulator::serializeNodesWithNamespaces(Node& targetNode, Serialize
         serializeNodesWithNamespaces(*current, SerializedNodes::SubtreeIncludingNode, &namespaceHash, tagNamesToSkip);
 
     if (root == SerializedNodes::SubtreeIncludingNode)
-        appendEndTag(targetNode);
+        endAppendingNode(targetNode);
 }
 
 String MarkupAccumulator::resolveURLIfNeeded(const Element& element, const String& urlString) const
@@ -185,22 +240,30 @@ void MarkupAccumulator::appendString(const String& string)
     m_markup.append(string);
 }
 
-void MarkupAccumulator::appendStartTag(const Node& node, Namespaces* namespaces)
+void MarkupAccumulator::appendStringView(StringView view)
 {
-    appendStartMarkup(m_markup, node, namespaces);
-    if (m_nodes)
-        m_nodes->append(const_cast<Node*>(&node));
+    m_markup.append(view);
 }
 
-void MarkupAccumulator::appendEndElement(StringBuilder& out, const Element& element)
+void MarkupAccumulator::startAppendingNode(const Node& node, Namespaces* namespaces)
 {
-    appendEndMarkup(out, element);
+    if (is<Element>(node))
+        appendStartTag(m_markup, downcast<Element>(node), namespaces);
+    else
+        appendNonElementNode(m_markup, node, namespaces);
+
+    if (m_nodes)
+        m_nodes->append(const_cast<Node*>(&node));
 }
 
-void MarkupAccumulator::appendTextSubstring(const Text& text, unsigned start, unsigned length)
+void MarkupAccumulator::appendEndTag(StringBuilder& result, const Element& element)
 {
-    ASSERT(start + length <= text.data().length());
-    appendCharactersReplacingEntities(m_markup, text.data(), start, length, entityMaskForText(text));
+    if (shouldSelfClose(element, m_serializationSyntax) || (!element.hasChildNodes() && elementCannotHaveEndTag(element)))
+        return;
+    result.append('<');
+    result.append('/');
+    result.append(element.nodeNamePreservingCase());
+    result.append('>');
 }
 
 size_t MarkupAccumulator::totalLength(const Vector<String>& strings)
@@ -410,7 +473,7 @@ void MarkupAccumulator::appendProcessingInstruction(StringBuilder& result, const
     result.append('>');
 }
 
-void MarkupAccumulator::appendElement(StringBuilder& result, const Element& element, Namespaces* namespaces)
+void MarkupAccumulator::appendStartTag(StringBuilder& result, const Element& element, Namespaces* namespaces)
 {
     appendOpenTag(result, element, namespaces);
 
@@ -444,7 +507,7 @@ void MarkupAccumulator::appendOpenTag(StringBuilder& result, const Element& elem
 
 void MarkupAccumulator::appendCloseTag(StringBuilder& result, const Element& element)
 {
-    if (shouldSelfClose(element)) {
+    if (shouldSelfClose(element, m_serializationSyntax)) {
         if (element.isHTMLElement())
             result.append(' '); // XHTML 1.0 <-> HTML compatibility.
         result.append('/');
@@ -530,7 +593,7 @@ void MarkupAccumulator::appendCDATASection(StringBuilder& result, const String&
     result.appendLiteral("]]>");
 }
 
-void MarkupAccumulator::appendStartMarkup(StringBuilder& result, const Node& node, Namespaces* namespaces)
+void MarkupAccumulator::appendNonElementNode(StringBuilder& result, const Node& node, Namespaces* namespaces)
 {
     if (namespaces)
         namespaces->checkConsistency();
@@ -554,7 +617,7 @@ void MarkupAccumulator::appendStartMarkup(StringBuilder& result, const Node& nod
         appendProcessingInstruction(result, downcast<ProcessingInstruction>(node).target(), downcast<ProcessingInstruction>(node).data());
         break;
     case Node::ELEMENT_NODE:
-        appendElement(result, downcast<Element>(node), namespaces);
+        ASSERT_NOT_REACHED();
         break;
     case Node::CDATA_SECTION_NODE:
         appendCDATASection(result, downcast<CDATASection>(node).data());
@@ -565,51 +628,4 @@ void MarkupAccumulator::appendStartMarkup(StringBuilder& result, const Node& nod
     }
 }
 
-// Rules of self-closure
-// 1. No elements in HTML documents use the self-closing syntax.
-// 2. Elements w/ children never self-close because they use a separate end tag.
-// 3. HTML elements which do not have a "forbidden" end tag will close with a separate end tag.
-// 4. Other elements self-close.
-bool MarkupAccumulator::shouldSelfClose(const Element& element)
-{
-    if (!inXMLFragmentSerialization() && element.document().isHTMLDocument())
-        return false;
-    if (element.hasChildNodes())
-        return false;
-    if (element.isHTMLElement() && !elementCannotHaveEndTag(element))
-        return false;
-    return true;
-}
-
-bool MarkupAccumulator::elementCannotHaveEndTag(const Node& node)
-{
-    if (!is<HTMLElement>(node))
-        return false;
-
-    // From https://html.spec.whatwg.org/#serialising-html-fragments:
-    // If current node is an area, base, basefont, bgsound, br, col, embed, frame, hr, img,
-    // input, keygen, link, meta, param, source, track or wbr element, then continue on to
-    // the next child node at this point.
-    static const HTMLQualifiedName* tags[] = { &areaTag.get(), &baseTag.get(), &basefontTag.get(), &bgsoundTag.get(),
-        &brTag.get(), &colTag.get(), &embedTag.get(), &frameTag.get(), &hrTag.get(), &imgTag.get(), &inputTag.get(),
-        &keygenTag.get(), &linkTag.get(), &metaTag.get(), &paramTag.get(), &sourceTag.get(), &trackTag.get(), &wbrTag.get() };
-    auto& element = downcast<HTMLElement>(node);
-    for (auto* tag : tags) {
-        if (element.hasTagName(*tag))
-            return true;
-    }
-    return false;
-}
-
-void MarkupAccumulator::appendEndMarkup(StringBuilder& result, const Element& element)
-{
-    if (shouldSelfClose(element) || (!element.hasChildNodes() && elementCannotHaveEndTag(element)))
-        return;
-
-    result.append('<');
-    result.append('/');
-    result.append(element.nodeNamePreservingCase());
-    result.append('>');
-}
-
 }
index 47f0743..aaacb27 100644 (file)
@@ -73,25 +73,24 @@ protected:
     void concatenateMarkup(StringBuilder&);
 
     void appendString(const String&);
-    void appendEndTag(const Node& node)
+    void appendStringView(StringView);
+
+    void startAppendingNode(const Node&, Namespaces* = nullptr);
+    void endAppendingNode(const Node& node)
     {
         if (is<Element>(node))
-            appendEndElement(m_markup, downcast<Element>(node));
+            appendEndTag(m_markup, downcast<Element>(node));
     }
 
-    virtual void appendEndElement(StringBuilder&, const Element&);
+    virtual void appendStartTag(StringBuilder&, const Element&, Namespaces*);
+    virtual void appendEndTag(StringBuilder&, const Element&);
     virtual void appendCustomAttributes(StringBuilder&, const Element&, Namespaces*);
     virtual void appendText(StringBuilder&, const Text&);
-    virtual void appendElement(StringBuilder&, const Element&, Namespaces*);
-
-    void appendStartTag(const Node&, Namespaces* = nullptr);
-
-    void appendTextSubstring(const Text&, unsigned start, unsigned length);
 
     void appendOpenTag(StringBuilder&, const Element&, Namespaces*);
     void appendCloseTag(StringBuilder&, const Element&);
 
-    void appendStartMarkup(StringBuilder&, const Node&, Namespaces*);
+    void appendNonElementNode(StringBuilder&, const Node&, Namespaces*);
     void appendEndMarkup(StringBuilder&, const Element&);
 
     void appendAttributeValue(StringBuilder&, const String&, bool isSerializingHTML);
@@ -104,8 +103,6 @@ protected:
 
     bool shouldAddNamespaceElement(const Element&);
     bool shouldAddNamespaceAttribute(const Attribute&, Namespaces&);
-    bool shouldSelfClose(const Element&);
-    bool elementCannotHaveEndTag(const Node&);
     EntityMask entityMaskForText(const Text&) const;
 
     Vector<Node*>* const m_nodes;
index 1729e0d..e40e3aa 100644 (file)
@@ -249,14 +249,14 @@ private:
 
     bool shouldPreserveMSOListStyleForElement(const Element&);
 
-    void appendElement(StringBuilder& out, const Element&, bool addDisplayInline, RangeFullySelectsNode);
-    void appendEndElement(StringBuilder& out, const Element&) override;
+    void appendStartTag(StringBuilder& out, const Element&, bool addDisplayInline, RangeFullySelectsNode);
+    void appendEndTag(StringBuilder& out, const Element&) override;
     void appendCustomAttributes(StringBuilder&, const Element&, Namespaces*) override;
 
     void appendText(StringBuilder& out, const Text&) override;
-    void appendElement(StringBuilder& out, const Element& element, Namespaces*) override
+    void appendStartTag(StringBuilder& out, const Element& element, Namespaces*) override
     {
-        appendElement(out, element, false, DoesFullySelectNode);
+        appendStartTag(out, element, false, DoesFullySelectNode);
     }
 
     Node* firstChild(Node& node)
@@ -347,11 +347,11 @@ void StyledMarkupAccumulator::wrapWithNode(Node& node, bool convertBlocksToInlin
 {
     StringBuilder markup;
     if (is<Element>(node))
-        appendElement(markup, downcast<Element>(node), convertBlocksToInlines && isBlock(&node), rangeFullySelectsNode);
+        appendStartTag(markup, downcast<Element>(node), convertBlocksToInlines && isBlock(&node), rangeFullySelectsNode);
     else
-        appendStartMarkup(markup, node, nullptr);
+        appendNonElementNode(markup, node, nullptr);
     m_reversedPrecedingMarkup.append(markup.toString());
-    appendEndTag(node);
+    endAppendingNode(node);
     if (m_nodes)
         m_nodes->append(&node);
 }
@@ -495,7 +495,7 @@ bool StyledMarkupAccumulator::shouldPreserveMSOListStyleForElement(const Element
     return false;
 }
 
-void StyledMarkupAccumulator::appendElement(StringBuilder& out, const Element& element, bool addDisplayInline, RangeFullySelectsNode rangeFullySelectsNode)
+void StyledMarkupAccumulator::appendStartTag(StringBuilder& out, const Element& element, bool addDisplayInline, RangeFullySelectsNode rangeFullySelectsNode)
 {
     const bool documentIsHTML = element.document().isHTMLDocument();
     const bool isSlotElement = is<HTMLSlotElement>(element);
@@ -563,12 +563,12 @@ void StyledMarkupAccumulator::appendElement(StringBuilder& out, const Element& e
     appendCloseTag(out, element);
 }
 
-void StyledMarkupAccumulator::appendEndElement(StringBuilder& out, const Element& element)
+void StyledMarkupAccumulator::appendEndTag(StringBuilder& out, const Element& element)
 {
     if (UNLIKELY(is<HTMLSlotElement>(element)))
         out.append("</span>");
     else
-        MarkupAccumulator::appendEndElement(out, element);
+        MarkupAccumulator::appendEndTag(out, element);
 }
 
 Node* StyledMarkupAccumulator::serializeNodes(const Position& start, const Position& end)
@@ -609,7 +609,7 @@ Node* StyledMarkupAccumulator::traverseNodesForSerialization(Node* startNode, No
 
         ++depth;
         if (shouldEmit)
-            appendStartTag(node);
+            startAppendingNode(node);
 
         return true;
     };
@@ -621,7 +621,7 @@ Node* StyledMarkupAccumulator::traverseNodesForSerialization(Node* startNode, No
             --depth;
         if (shouldEmit) {
             if (closing)
-                appendEndTag(node);
+                endAppendingNode(node);
             else
                 wrapWithNode(node);
         }
@@ -689,7 +689,7 @@ bool StyledMarkupAccumulator::appendNodeToPreserveMSOList(Node& node)
             m_inMSOList = false;
         else
             return false;
-        appendStartTag(commentNode);
+        startAppendingNode(commentNode);
         return true;
     }
     if (is<HTMLStyleElement>(node)) {
@@ -712,7 +712,7 @@ bool StyledMarkupAccumulator::appendNodeToPreserveMSOList(Node& node)
             return false;
 
         appendString("<head><style class=\"" WebKitMSOListQuirksStyle "\">\n<!--\n");
-        appendTextSubstring(textChild, start, msoListDefinitionsEnd - start + 3);
+        appendStringView(StringView(textChild.data()).substring(start, msoListDefinitionsEnd - start + 3));
         appendString("\n-->\n</style></head>");
 
         return true;
index 3caa382..a3b43a0 100644 (file)
@@ -103,9 +103,9 @@ private:
     Document& m_document;
 
     void appendText(StringBuilder&, const Text&) override;
-    void appendElement(StringBuilder&, const Element&, Namespaces*) override;
+    void appendStartTag(StringBuilder&, const Element&, Namespaces*) override;
     void appendCustomAttributes(StringBuilder&, const Element&, Namespaces*) override;
-    void appendEndElement(StringBuilder&, const Element&) override;
+    void appendEndTag(StringBuilder&, const Element&) override;
 };
 
 PageSerializer::SerializerMarkupAccumulator::SerializerMarkupAccumulator(PageSerializer& serializer, Document& document, Vector<Node*>* nodes)
@@ -125,10 +125,10 @@ void PageSerializer::SerializerMarkupAccumulator::appendText(StringBuilder& out,
         MarkupAccumulator::appendText(out, text);
 }
 
-void PageSerializer::SerializerMarkupAccumulator::appendElement(StringBuilder& out, const Element& element, Namespaces* namespaces)
+void PageSerializer::SerializerMarkupAccumulator::appendStartTag(StringBuilder& out, const Element& element, Namespaces* namespaces)
 {
     if (!shouldIgnoreElement(element))
-        MarkupAccumulator::appendElement(out, element, namespaces);
+        MarkupAccumulator::appendStartTag(out, element, namespaces);
 
     if (element.hasTagName(HTMLNames::headTag)) {
         out.appendLiteral("<meta charset=\"");
@@ -158,10 +158,10 @@ void PageSerializer::SerializerMarkupAccumulator::appendCustomAttributes(StringB
     appendAttribute(out, element, Attribute(frameOwnerURLAttributeName(frameOwner), url.string()), namespaces);
 }
 
-void PageSerializer::SerializerMarkupAccumulator::appendEndElement(StringBuilder& out, const Element& element)
+void PageSerializer::SerializerMarkupAccumulator::appendEndTag(StringBuilder& out, const Element& element)
 {
     if (!shouldIgnoreElement(element))
-        MarkupAccumulator::appendEndElement(out, element);
+        MarkupAccumulator::appendEndTag(out, element);
 }
 
 PageSerializer::PageSerializer(Vector<PageSerializer::Resource>& resources)