Bug 1269527 - stop showing incorrect closing tagline for #document;r=bgrins draft
authorJulian Descottes <jdescottes@mozilla.com>
Thu, 25 Jan 2018 23:07:11 +0100
changeset 749563 ae19efe422f255a08dc18f8a9044ea85c7a6bba3
parent 749562 92e628b358ce1a92fcdaf617f49c47116df4e7e6
child 749565 b36799130e66e4abdaee628d48ad1b056bc1afcd
push id97436
push userjdescottes@mozilla.com
push dateWed, 31 Jan 2018 18:02:50 +0000
reviewersbgrins
bugs1269527
milestone60.0a1
Bug 1269527 - stop showing incorrect closing tagline for #document;r=bgrins MozReview-Commit-ID: 9htiEEWlCTG
devtools/client/inspector/markup/test/browser.ini
devtools/client/inspector/markup/test/browser_markup_toggle_closing_tag_line.js
devtools/client/inspector/markup/test/head.js
devtools/client/inspector/markup/views/markup-container.js
--- a/devtools/client/inspector/markup/test/browser.ini
+++ b/devtools/client/inspector/markup/test/browser.ini
@@ -181,12 +181,13 @@ skip-if = e10s # Bug 1036409 - The last 
 [browser_markup_tag_edit_avoid_refocus.js]
 [browser_markup_tag_edit_long-classname.js]
 [browser_markup_textcontent_display.js]
 [browser_markup_textcontent_edit_01.js]
 [browser_markup_textcontent_edit_02.js]
 [browser_markup_toggle_01.js]
 [browser_markup_toggle_02.js]
 [browser_markup_toggle_03.js]
+[browser_markup_toggle_closing_tag_line.js]
 [browser_markup_update-on-navigtion.js]
 [browser_markup_void_elements_html.js]
 [browser_markup_void_elements_xhtml.js]
 [browser_markup_whitespace.js]
new file mode 100644
--- /dev/null
+++ b/devtools/client/inspector/markup/test/browser_markup_toggle_closing_tag_line.js
@@ -0,0 +1,45 @@
+/* vim: set ts=2 et sw=2 tw=80: */
+/* Any copyright is dedicated to the Public Domain.
+ http://creativecommons.org/publicdomain/zero/1.0/ */
+
+"use strict";
+
+// Test that a closing tag line is displayed when expanding an element container.
+// Also check that no closing tag line is displayed for readonly containers (document,
+// roots...).
+
+const TEST_URL = `data:text/html;charset=utf8,
+<div class="outer-div"><span>test</span></div>
+<iframe src="data:text/html;charset=utf8,<div>test</div>"></iframe>`;
+
+add_task(async function () {
+  let {inspector} = await openInspectorForURL(TEST_URL);
+
+  info("Getting the container for .outer-div parent element");
+  let container = await getContainerForSelector(".outer-div", inspector);
+  await expandContainer(inspector, container);
+
+  let closeTagLine = container.closeTagLine;
+  ok(closeTagLine && closeTagLine.textContent.includes("div"),
+     "DIV has a close tag-line with the correct content");
+
+  info("Expand the iframe element");
+  container = await getContainerForSelector("iframe", inspector);
+  await expandContainer(inspector, container);
+  ok(container.expanded, "iframe is expanded");
+  closeTagLine = container.closeTagLine;
+  ok(closeTagLine && closeTagLine.textContent.includes("iframe"),
+     "IFRAME has a close tag-line with the correct content");
+
+  info("Retrieve the nodefront for the #document root inside the iframe");
+  let iframe = await getNodeFront("iframe", inspector);
+  let {nodes} = await inspector.walker.children(iframe);
+  let documentFront = nodes[0];
+  ok(documentFront.displayName === "#document", "First child of IFRAME is #document");
+
+  info("Expand the iframe's #document node element");
+  container = getContainerForNodeFront(documentFront, inspector);
+  await expandContainer(inspector, container);
+  ok(container.expanded, "#document is expanded");
+  ok(!container.closeTagLine, "readonly (#document) node has no close tag-line");
+});
--- a/devtools/client/inspector/markup/test/head.js
+++ b/devtools/client/inspector/markup/test/head.js
@@ -629,8 +629,21 @@ function* checkDeleteAndSelection(inspec
   let node = yield getNodeFront(selector, inspector);
   ok(!node, "The node can't be found in the page anymore");
 
   info("Undo the deletion to restore the original markup");
   yield undoChange(inspector);
   node = yield getNodeFront(selector, inspector);
   ok(node, "The node is back");
 }
+
+/**
+ * Expand the provided markup container by clicking on the expand arrow and waiting for
+ * inspector and children to update.
+ */
+async function expandContainer(inspector, container) {
+  let onChildren = waitForChildrenUpdated(inspector);
+  let onUpdated = inspector.once("inspector-updated");
+  EventUtils.synthesizeMouseAtCenter(container.expander, {},
+    inspector.markup.doc.defaultView);
+  await onChildren;
+  await onUpdated;
+}
--- a/devtools/client/inspector/markup/views/markup-container.js
+++ b/devtools/client/inspector/markup/views/markup-container.js
@@ -6,16 +6,22 @@
 
 const {Task} = require("devtools/shared/task");
 const {KeyCodes} = require("devtools/client/shared/keycodes");
 const {flashElementOn, flashElementOff} =
       require("devtools/client/inspector/markup/utils");
 
 const DRAG_DROP_MIN_INITIAL_DISTANCE = 10;
 
+const TYPES = {
+  TEXT_CONTAINER: "textcontainer",
+  ELEMENT_CONTAINER: "elementcontainer",
+  READ_ONLY_CONTAINER: "readonlycontainer",
+};
+
 /**
  * The main structure for storing a document node in the markup
  * tree.  Manages creation of the editor for the node and
  * a <ul> for placing child elements, and expansion/collapsing
  * of the element.
  *
  * This should not be instantiated directly, instead use one of:
  *    MarkupReadOnlyContainer
@@ -35,28 +41,29 @@ MarkupContainer.prototype = {
    * Initialize the MarkupContainer.  Should be called while one
    * of the other contain classes is instantiated.
    *
    * @param  {MarkupView} markupView
    *         The markup view that owns this container.
    * @param  {NodeFront} node
    *         The node to display.
    * @param  {String} type
-   *         The type of container to build. This can be either 'textcontainer',
-   *         'readonlycontainer' or 'elementcontainer'.
+   *         The type of container to build. One of TYPES.TEXT_CONTAINER,
+   *         TYPES.ELEMENT_CONTAINER, TYPES.READ_ONLY_CONTAINER
    */
   initialize: function (markupView, node, type) {
     this.markup = markupView;
     this.node = node;
+    this.type = type;
     this.undo = this.markup.undo;
     this.win = this.markup._frame.contentWindow;
     this.id = "treeitem-" + markupContainerID++;
     this.htmlElt = this.win.document.documentElement;
 
-    this.buildMarkup(type);
+    this.buildMarkup();
 
     this.elt.container = this;
 
     this._onMouseDown = this._onMouseDown.bind(this);
     this._onToggle = this._onToggle.bind(this);
     this._onKeyDown = this._onKeyDown.bind(this);
 
     // Binding event listeners
@@ -65,17 +72,17 @@ MarkupContainer.prototype = {
     if (this.expander) {
       this.expander.addEventListener("click", this._onToggle);
     }
 
     // Marking the node as shown or hidden
     this.updateIsDisplayed();
   },
 
-  buildMarkup: function (type) {
+  buildMarkup: function () {
     this.elt = this.win.document.createElement("li");
     this.elt.classList.add("child", "collapsed");
     this.elt.setAttribute("role", "presentation");
 
     this.tagLine = this.win.document.createElement("div");
     this.tagLine.setAttribute("id", this.id);
     this.tagLine.classList.add("tag-line");
     this.tagLine.setAttribute("role", "treeitem");
@@ -83,17 +90,17 @@ MarkupContainer.prototype = {
     this.tagLine.setAttribute("aria-grabbed", this.isDragging);
     this.elt.appendChild(this.tagLine);
 
     this.tagState = this.win.document.createElement("span");
     this.tagState.classList.add("tag-state");
     this.tagState.setAttribute("role", "presentation");
     this.tagLine.appendChild(this.tagState);
 
-    if (type !== "textcontainer") {
+    if (this.type !== TYPES.TEXT_CONTAINER) {
       this.expander = this.win.document.createElement("span");
       this.expander.classList.add("theme-twisty", "expander");
       this.expander.setAttribute("role", "presentation");
       this.tagLine.appendChild(this.expander);
     }
 
     this.children = this.win.document.createElement("ul");
     this.children.classList.add("children");
@@ -298,61 +305,89 @@ MarkupContainer.prototype = {
   setExpanded: function (value) {
     if (!this.expander) {
       return;
     }
 
     if (!this.canExpand) {
       value = false;
     }
+
     if (this.mustExpand) {
       value = true;
     }
 
     if (value && this.elt.classList.contains("collapsed")) {
-      // Expanding a node means cloning its "inline" closing tag into a new
-      // tag-line that the user can interact with and showing the children.
-      let closingTag = this.elt.querySelector(".close");
-      if (closingTag) {
-        if (!this.closeTagLine) {
-          let line = this.markup.doc.createElement("div");
-          line.classList.add("tag-line");
-          // Closing tag is not important for accessibility.
-          line.setAttribute("role", "presentation");
-
-          let tagState = this.markup.doc.createElement("div");
-          tagState.classList.add("tag-state");
-          line.appendChild(tagState);
-
-          line.appendChild(closingTag.cloneNode(true));
-
-          flashElementOff(line);
-          this.closeTagLine = line;
-        }
-        this.elt.appendChild(this.closeTagLine);
-      }
+      this.showCloseTagLine();
 
       this.elt.classList.remove("collapsed");
       this.expander.setAttribute("open", "");
       this.hovered = false;
       this.markup.emit("expanded");
     } else if (!value) {
-      if (this.closeTagLine) {
-        this.elt.removeChild(this.closeTagLine);
-        this.closeTagLine = undefined;
-      }
+      this.hideCloseTagLine();
+
       this.elt.classList.add("collapsed");
       this.expander.removeAttribute("open");
       this.markup.emit("collapsed");
     }
+
     if (this.showExpander) {
       this.tagLine.setAttribute("aria-expanded", this.expanded);
     }
   },
 
+  /**
+   * Expanding a node means cloning its "inline" closing tag into a new
+   * tag-line that the user can interact with and showing the children.
+   */
+  showCloseTagLine: function () {
+    // Only element containers display a closing tag line. #document has no closing line.
+    if (this.type !== TYPES.ELEMENT_CONTAINER) {
+      return;
+    }
+
+    // Retrieve the closest .close node for this container.
+    let closingTag = this.elt.querySelector(".close");
+    if (!closingTag) {
+      return;
+    }
+
+    // Create the closing tag-line element if not already created.
+    if (!this.closeTagLine) {
+      let line = this.markup.doc.createElement("div");
+      line.classList.add("tag-line");
+      // Closing tag is not important for accessibility.
+      line.setAttribute("role", "presentation");
+
+      let tagState = this.markup.doc.createElement("div");
+      tagState.classList.add("tag-state");
+      line.appendChild(tagState);
+
+      line.appendChild(closingTag.cloneNode(true));
+
+      flashElementOff(line);
+      this.closeTagLine = line;
+    }
+    this.elt.appendChild(this.closeTagLine);
+  },
+
+  /**
+   * Hide the closing tag-line element which should only be displayed when the container
+   * is expanded.
+   */
+  hideCloseTagLine: function () {
+    if (!this.closeTagLine) {
+      return;
+    }
+
+    this.elt.removeChild(this.closeTagLine);
+    this.closeTagLine = undefined;
+  },
+
   parentContainer: function () {
     return this.elt.parentNode ? this.elt.parentNode.container : null;
   },
 
   /**
    * Determine tree depth level of a given node. This is used to specify ARIA
    * level for node tree items and to give them better semantic context.
    */