Bug 1575620 - Fix refreshing session restore when using prototype cache. r=smaug

The session restore page keeps its restore list within a text input field so that the values are persisted even if the page is refreshed. When form elements were loaded with the prototype cache we didn't call DoneCreatingElement after creating the element, which means the form values weren't restored. The list of elements that require DoneCreatingElement and DoneAddingChildren to be called was in three (now four) different places, so I moved them to a central spot in nsIContent to share in all locations. This also highlighted that the check for <output> nodes is missing from the XML content sink. Differential Revision: https://phabricator.services.mozilla.com/D44866 --HG-- extra : moz-landing-system : lando
2019-09-05 22:26:25 +00:00 · 2019-09-05 22:26:25 +00:00 · c377fd351d
--- a/dom/base/nsIContent.h
+++ b/dom/base/nsIContent.h
@ -542,9 +542,8 @@ class nsIContent : public nsINode {
   * For container elements, this is called *before* any of the children are
   * created or added into the tree.
   *
-   * NOTE: this is currently only called for input and button, in the HTML
-   * content sink.  If you want to call it on your element, modify the content
-   * sink of your choice to do so.  This is an efficiency measure.
+   * NOTE: this is only called for elements listed in
+   * RequiresDoneCreatingElement. This is an efficiency measure.
   *
   * If you also need to determine whether the parser is the one creating your
   * element (through createElement() or cloneNode() generally) then add a
@ -565,10 +564,8 @@ class nsIContent : public nsINode {
   * This method is called when the parser finishes creating the element's
   * children, if any are present.
   *
-   * NOTE: this is currently only called for textarea, select, and object
-   * elements in the HTML content sink. If you want to call it on your element,
-   * modify the content sink of your choice to do so. This is an efficiency
-   * measure.
+   * NOTE: this is only called for elements listed in
+   * RequiresDoneAddingChildren. This is an efficiency measure.
   *
   * If you also need to determine whether the parser is the one creating your
   * element (through createElement() or cloneNode() generally) then add a
@ -595,6 +592,47 @@ class nsIContent : public nsINode {
   */
  virtual bool IsDoneAddingChildren() { return true; }

+  /**
+   * Returns true if an element needs its DoneCreatingElement method to be
+   * called after it has been created.
+   * @see nsIContent::DoneCreatingElement
+   *
+   * @param aNamespaceID the node's namespace ID
+   * @param aName the node's tag name
+   */
+  static inline bool RequiresDoneCreatingElement(int32_t aNamespace,
+                                                 nsAtom* aName) {
+    if (aNamespace == kNameSpaceID_XHTML &&
+        (aName == nsGkAtoms::input || aName == nsGkAtoms::button ||
+         aName == nsGkAtoms::menuitem || aName == nsGkAtoms::audio ||
+         aName == nsGkAtoms::video)) {
+      MOZ_ASSERT(!RequiresDoneAddingChildren(
+          aNamespace, aName,
+          "Both DoneCreatingElement and DoneAddingChildren on a same element "
+          "isn't supported."));
+      return true;
+    }
+    return false;
+  }
+
+  /**
+   * Returns true if an element needs its DoneAddingChildren method to be
+   * called after all of its children have been added.
+   * @see nsIContent::DoneAddingChildren
+   *
+   * @param aNamespace the node's namespace ID
+   * @param aName the node's tag name
+   */
+  static inline bool RequiresDoneAddingChildren(int32_t aNamespace,
+                                                nsAtom* aName) {
+    return (aNamespace == kNameSpaceID_XHTML &&
+            (aName == nsGkAtoms::select || aName == nsGkAtoms::textarea ||
+             aName == nsGkAtoms::head || aName == nsGkAtoms::title ||
+             aName == nsGkAtoms::object || aName == nsGkAtoms::output)) ||
+           (aNamespace == kNameSpaceID_SVG && aName == nsGkAtoms::title) ||
+           (aNamespace == kNameSpaceID_XUL && aName == nsGkAtoms::linkset);
+  }
+
  /**
   * Get the ID of this content node (the atom corresponding to the
   * value of the id attribute).  This may be null if there is no ID.
--- a/dom/prototype/PrototypeDocumentContentSink.cpp
+++ b/dom/prototype/PrototypeDocumentContentSink.cpp
@ -410,7 +410,9 @@ nsresult PrototypeDocumentContentSink::InsertXMLStylesheetPI(
 }

 void PrototypeDocumentContentSink::CloseElement(Element* aElement) {
-  if (aElement->IsXULElement(nsGkAtoms::linkset)) {
+  if (nsIContent::RequiresDoneAddingChildren(
+          aElement->NodeInfo()->NamespaceID(),
+          aElement->NodeInfo()->NameAtom())) {
    aElement->DoneAddingChildren(false);
  }
 }
@ -512,6 +514,12 @@ nsresult PrototypeDocumentContentSink::ResumeWalkInternal() {
          rv = nodeToPushTo->AppendChildTo(child, false);
          if (NS_FAILED(rv)) return rv;

+          if (nsIContent::RequiresDoneCreatingElement(
+                  protoele->mNodeInfo->NamespaceID(),
+                  protoele->mNodeInfo->NameAtom())) {
+            child->DoneCreatingElement();
+          }
+
          // If it has children, push the element onto the context
          // stack and begin to process them.
          if (protoele->mChildren.Length() > 0) {
--- a/dom/prototype/tests/chrome/chrome.ini
+++ b/dom/prototype/tests/chrome/chrome.ini
@ -2,5 +2,6 @@
 support-files =
  whitespace.xhtml
  no_whitespace.xhtml
+  form.xhtml

 [test_prototype_document.xhtml]
--- a/dom/prototype/tests/chrome/form.xhtml
+++ b/dom/prototype/tests/chrome/form.xhtml
@ -0,0 +1,8 @@
+<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0 Transitional//EN"
+"http://www.w3.org/TR/xhtml1/DTD/xhtml1-transitional.dtd">
+
+<html xmlns="http://www.w3.org/1999/xhtml">
+  <body>
+    <input type="text" id="input" value=""/>
+  </body>
+</html>
--- a/dom/prototype/tests/chrome/test_prototype_document.xhtml
+++ b/dom/prototype/tests/chrome/test_prototype_document.xhtml
@ -61,6 +61,15 @@ add_task(async function test_prototype_document() {
  // TODO: Test whitespace within XUL elements, since it is handled differently
  // with and without the prototype sink (bug 1544567).
 });
+
+add_task(async function test_prototype_document_form() {
+  let browser = document.getElementById("browser");
+  await load(browser, "form.xhtml");
+  ok(browser.contentDocument.loadedFromPrototype, `form.xhtml should load from prototype.`);
+  browser.contentDocument.getElementById("input").value = "test";
+  await load(browser, "form.xhtml");
+  is(browser.contentDocument.getElementById("input").value, "test", "input value should persist after reload");
+});
 ]]></script>
 </body>
 </window>
--- a/dom/xml/nsXMLContentSink.cpp
+++ b/dom/xml/nsXMLContentSink.cpp
@ -522,14 +522,8 @@ nsresult nsXMLContentSink::CloseElement(nsIContent* aContent) {

  // Some HTML nodes need DoneAddingChildren() called to initialize
  // properly (eg form state restoration).
-  if ((nodeInfo->NamespaceID() == kNameSpaceID_XHTML &&
-       (nodeInfo->NameAtom() == nsGkAtoms::select ||
-        nodeInfo->NameAtom() == nsGkAtoms::textarea ||
-        nodeInfo->NameAtom() == nsGkAtoms::video ||
-        nodeInfo->NameAtom() == nsGkAtoms::audio ||
-        nodeInfo->NameAtom() == nsGkAtoms::head ||
-        nodeInfo->NameAtom() == nsGkAtoms::object)) ||
-      nodeInfo->NameAtom() == nsGkAtoms::title) {
+  if (nsIContent::RequiresDoneAddingChildren(nodeInfo->NamespaceID(),
+                                             nodeInfo->NameAtom())) {
    aContent->DoneAddingChildren(HaveNotifiedForCurrentContent());
  }

@ -957,16 +951,14 @@ nsresult nsXMLContentSink::HandleStartElement(

  // Some HTML nodes need DoneCreatingElement() called to initialize
  // properly (eg form state restoration).
-  if (nodeInfo->NamespaceID() == kNameSpaceID_XHTML) {
-    if (nodeInfo->NameAtom() == nsGkAtoms::input ||
-        nodeInfo->NameAtom() == nsGkAtoms::button ||
-        nodeInfo->NameAtom() == nsGkAtoms::menuitem ||
-        nodeInfo->NameAtom() == nsGkAtoms::audio ||
-        nodeInfo->NameAtom() == nsGkAtoms::video) {
-      content->DoneCreatingElement();
-    } else if (nodeInfo->NameAtom() == nsGkAtoms::head && !mCurrentHead) {
-      mCurrentHead = content;
-    }
+  if (nsIContent::RequiresDoneCreatingElement(nodeInfo->NamespaceID(),
+                                              nodeInfo->NameAtom())) {
+    content->DoneCreatingElement();
+  }
+
+  if (nodeInfo->NamespaceID() == kNameSpaceID_XHTML &&
+      nodeInfo->NameAtom() == nsGkAtoms::head && !mCurrentHead) {
+    mCurrentHead = content;
  }

  if (IsMonolithicContainer(nodeInfo)) {
--- a/dom/xslt/xslt/txMozillaXMLOutput.cpp
+++ b/dom/xslt/xslt/txMozillaXMLOutput.cpp
@ -262,10 +262,10 @@ nsresult txMozillaXMLOutput::endElement() {
    }

    // Handle elements that are different when parser-created
-    if (element->IsAnyOfHTMLElements(nsGkAtoms::title, nsGkAtoms::object,
-                                     nsGkAtoms::select, nsGkAtoms::textarea) ||
-        element->IsSVGElement(nsGkAtoms::title)) {
-      element->DoneAddingChildren(true);
+    if (nsIContent::RequiresDoneCreatingElement(
+            element->NodeInfo()->NamespaceID(),
+            element->NodeInfo()->NameAtom())) {
+      element->DoneCreatingElement();
    } else if (element->IsSVGElement(nsGkAtoms::script) ||
               element->IsHTMLElement(nsGkAtoms::script)) {
      nsCOMPtr<nsIScriptElement> sele = do_QueryInterface(element);
@ -282,10 +282,10 @@ nsresult txMozillaXMLOutput::endElement() {
                   "Script elements need to implement nsIScriptElement and SVG "
                   "wasn't disabled.");
      }
-    } else if (element->IsAnyOfHTMLElements(
-                   nsGkAtoms::input, nsGkAtoms::button, nsGkAtoms::menuitem,
-                   nsGkAtoms::audio, nsGkAtoms::video)) {
-      element->DoneCreatingElement();
+    } else if (nsIContent::RequiresDoneAddingChildren(
+                   element->NodeInfo()->NamespaceID(),
+                   element->NodeInfo()->NameAtom())) {
+      element->DoneAddingChildren(true);
    }
  }

--- a/parser/html/nsHtml5TreeBuilderCppSupplement.h
+++ b/parser/html/nsHtml5TreeBuilderCppSupplement.h
@ -905,18 +905,7 @@ void nsHtml5TreeBuilder::elementPushed(int32_t aNamespace, nsAtom* aName,
    treeOp->Init(mozilla::AsVariant(opStartLayout()));
    return;
  }
-  if (aName == nsGkAtoms::input || aName == nsGkAtoms::button) {
-    if (mBuilder) {
-      nsHtml5TreeOperation::DoneCreatingElement(
-          static_cast<nsIContent*>(aElement));
-    } else {
-      opDoneCreatingElement operation(aElement);
-      mOpQueue.AppendElement()->Init(mozilla::AsVariant(operation));
-    }
-    return;
-  }
-  if (aName == nsGkAtoms::audio || aName == nsGkAtoms::video ||
-      aName == nsGkAtoms::menuitem) {
+  if (nsIContent::RequiresDoneCreatingElement(kNameSpaceID_XHTML, aName)) {
    if (mBuilder) {
      nsHtml5TreeOperation::DoneCreatingElement(
          static_cast<nsIContent*>(aElement));
@ -984,7 +973,9 @@ void nsHtml5TreeBuilder::elementPopped(int32_t aNamespace, nsAtom* aName,
    treeOp->Init(mozilla::AsVariant(operation));
    return;
  }
-  if (aName == nsGkAtoms::title) {
+  // Some nodes need DoneAddingChildren() called to initialize
+  // properly (e.g. form state restoration).
+  if (nsIContent::RequiresDoneAddingChildren(aNamespace, aName)) {
    if (mBuilder) {
      nsHtml5TreeOperation::DoneAddingChildren(
          static_cast<nsIContent*>(aElement));
@ -1033,26 +1024,6 @@ void nsHtml5TreeBuilder::elementPopped(int32_t aNamespace, nsAtom* aName,
    return;
  }
  // we now have only HTML
-  // Some HTML nodes need DoneAddingChildren() called to initialize
-  // properly (e.g. form state restoration).
-  // XXX expose ElementName group here and do switch
-  if (aName == nsGkAtoms::object || aName == nsGkAtoms::select ||
-      aName == nsGkAtoms::textarea || aName == nsGkAtoms::output ||
-      aName == nsGkAtoms::head) {
-    if (mBuilder) {
-      nsHtml5TreeOperation::DoneAddingChildren(
-          static_cast<nsIContent*>(aElement));
-      return;
-    }
-    nsHtml5TreeOperation* treeOp = mOpQueue.AppendElement(mozilla::fallible);
-    if (MOZ_UNLIKELY(!treeOp)) {
-      MarkAsBrokenAndRequestSuspensionWithoutBuilder(NS_ERROR_OUT_OF_MEMORY);
-      return;
-    }
-    opDoneAddingChildren operation(aElement);
-    treeOp->Init(mozilla::AsVariant(operation));
-    return;
-  }
  if (aName == nsGkAtoms::meta && !fragment && !mBuilder) {
    nsHtml5TreeOperation* treeOp = mOpQueue.AppendElement(mozilla::fallible);
    if (MOZ_UNLIKELY(!treeOp)) {