Use tighter typing in editing: MarkupAccumulator

Source/core/editing/MarkupAccumulator.cpp

 /*
  * Copyright (C) 2004, 2005, 2006, 2007, 2008, 2009, 2012 Apple Inc. All rights reserved.
  * Copyright (C) 2009, 2010 Google Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
  * are met:
  * 1. Redistributions of source code must retain the above copyright
  *    notice, this list of conditions and the following disclaimer.
  * 2. Redistributions in binary form must reproduce the above copyright
@@ skipping 119 matching lines @@
             if (toElement(targetNode).hasTagName(tagNamesToSkip->at(i)))
                 return;
         }
     }
 
     Namespaces namespaceHash;
     if (namespaces)
         namespaceHash = *namespaces;
 
     if (!childrenOnly)
         appendStartTag(targetNode, &namespaceHash);

[yosin_UTC9, 2014/08/01 02:27:14]

We may want to pass |Element*| to |appendStartTag|.

[Inactive, 2014/08/01 04:08:12]

I actually had the same idea but had to revert that part of the change because
appendStartTag() does more than appending the start tag. appendStartTag() calls
MarkupAccumulator::appendStartMarkup(), which serializes all types of nodes
(Text, Comment, Attr, ...), not just Elements.

I think this would need further refactoring / renaming to clarify.

 
     if (!(serializeAsHTMLDocument(targetNode) && elementCannotHaveEndTag(targetNode))) {
         Node* current = isHTMLTemplateElement(targetNode) ? toHTMLTemplateElement(targetNode).content()->firstChild() : targetNode.firstChild();
         for ( ; current; current = current->nextSibling())
             serializeNodesWithNamespaces(*current, IncludeNode, &namespaceHash, tagNamesToSkip);
     }
 
-    if (!childrenOnly)
-        appendEndTag(targetNode);
+    if (!childrenOnly && targetNode.isElementNode())
+        appendEndTag(toElement(targetNode));

[yosin_UTC9, 2014/08/01 02:27:14]

Good catch!

 }
 
 String MarkupAccumulator::resolveURLIfNeeded(const Element& element, const String& urlString) const
 {
     switch (m_resolveURLsMethod) {
     case ResolveAllURLs:
         return element.document().completeURL(urlString).string();
 
     case ResolveNonLocalURLs:
         if (!element.document().url().isLocalFile())
@@ skipping 11 matching lines @@
     m_markup.append(string);
 }
 
 void MarkupAccumulator::appendStartTag(Node& node, Namespaces* namespaces)
 {
     appendStartMarkup(m_markup, node, namespaces);
     if (m_nodes)
         m_nodes->append(&node);
 }
 
-void MarkupAccumulator::appendEndTag(const Node& node)
+void MarkupAccumulator::appendEndTag(const Element& element)
 {
-    appendEndMarkup(m_markup, node);
+    appendEndMarkup(m_markup, element);
 }
 
 size_t MarkupAccumulator::totalLength(const Vector<String>& strings)
 {
     size_t length = 0;
     for (size_t i = 0; i < strings.size(); ++i)
         length += strings[i].length();
     return length;
 }
 
@@ skipping 197 matching lines @@
         for (AttributeCollection::const_iterator it = attributes.begin(); it != end; ++it)
             appendAttribute(result, element, *it, namespaces);
     }
 
     // Give an opportunity to subclasses to add their own attributes.
     appendCustomAttributes(result, element, namespaces);
 
     appendCloseTag(result, element);
 }
 
-static String nodeNamePreservingCase(const Element& element)
-{
-    return element.tagQName().toString();
-}
-
 void MarkupAccumulator::appendOpenTag(StringBuilder& result, const Element& element, Namespaces* namespaces)
 {
     result.append('<');
-    result.append(nodeNamePreservingCase(element));
+    result.append(element.tagQName().toString());
     if (!serializeAsHTMLDocument(element) && namespaces && shouldAddNamespaceElement(element, *namespaces))
         appendNamespace(result, element.prefix(), element.namespaceURI(), *namespaces);
 }
 
 void MarkupAccumulator::appendCloseTag(StringBuilder& result, const Element& element)
 {
     if (shouldSelfClose(element)) {
         if (element.isHTMLElement())
             result.append(' '); // XHTML 1.0 <-> HTML compatibility.
         result.append('/');
@@ skipping 100 matching lines @@
         ASSERT_NOT_REACHED();
         break;
     }
 }
 
 // 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 Node& node)
+bool MarkupAccumulator::shouldSelfClose(const Element& element)
 {
-    if (serializeAsHTMLDocument(node))
+    if (serializeAsHTMLDocument(element))
         return false;
-    if (node.hasChildren())
+    if (element.hasChildren())
         return false;
-    if (node.isHTMLElement() && !elementCannotHaveEndTag(node))
+    if (element.isHTMLElement() && !elementCannotHaveEndTag(element))
         return false;
     return true;
 }
 
 bool MarkupAccumulator::elementCannotHaveEndTag(const Node& node)
 {
     if (!node.isHTMLElement())
         return false;
 
     // FIXME: ieForbidsInsertHTML may not be the right function to call here
     // ieForbidsInsertHTML is used to disallow setting innerHTML/outerHTML
     // or createContextualFragment.  It does not necessarily align with
     // which elements should be serialized w/o end tags.
     return toHTMLElement(node).ieForbidsInsertHTML();
 }
 
-void MarkupAccumulator::appendEndMarkup(StringBuilder& result, const Node& node)
+void MarkupAccumulator::appendEndMarkup(StringBuilder& result, const Element& element)
 {
-    if (!node.isElementNode() || shouldSelfClose(node) || (!node.hasChildren() && elementCannotHaveEndTag(node)))
+    if (shouldSelfClose(element) || (!element.hasChildren() && elementCannotHaveEndTag(element)))
         return;
 
     result.appendLiteral("</");
-    result.append(nodeNamePreservingCase(toElement(node)));
+    result.append(element.tagQName().toString());
     result.append('>');
 }
 
 bool MarkupAccumulator::serializeAsHTMLDocument(const Node& node) const
 {
     if (m_serializationType == ForcedXML)
         return false;
     return node.document().isHTMLDocument();
 }
 
 }