Bug 987877 - Add Copy XPath menu item to the inspector; r=miker
authorPatrick Brosset <pbrosset@mozilla.com>
Mon, 12 Jun 2017 16:25:48 +0200
changeset 412286 4f7877809d7973fbc9e7d37f74039d4598ec384a
parent 412285 62c2763b9dd6f92aaa805c56776f4515620d1d9b
child 412287 c8f3df191e354062025352a45ac421e557ed324c
child 412340 5c0b6ba1da0667d4c9fbea57d2b1d64344836ff5
push id7566
push usermtabara@mozilla.com
push dateWed, 02 Aug 2017 08:25:16 +0000
treeherdermozilla-beta@86913f512c3c [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmiker
bugs987877
milestone56.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 987877 - Add Copy XPath menu item to the inspector; r=miker MozReview-Commit-ID: A5g0MmWovjk
devtools/client/inspector/inspector.js
devtools/client/inspector/test/browser_inspector_menu-01-sensitivity.js
devtools/client/inspector/test/browser_inspector_menu-02-copy-items.js
devtools/client/locales/en-US/inspector.properties
devtools/client/shared/telemetry.js
devtools/server/actors/inspector.js
devtools/server/actors/root.js
devtools/shared/inspector/css-logic.js
devtools/shared/specs/node.js
devtools/shared/tests/mochitest/chrome.ini
devtools/shared/tests/mochitest/test_css-logic-getXPath.html
toolkit/components/telemetry/Scalars.yaml
--- a/devtools/client/inspector/inspector.js
+++ b/devtools/client/inspector/inspector.js
@@ -176,16 +176,20 @@ Inspector.prototype = {
   get canGetUniqueSelector() {
     return this._target.client.traits.getUniqueSelector;
   },
 
   get canGetCssPath() {
     return this._target.client.traits.getCssPath;
   },
 
+  get canGetXPath() {
+    return this._target.client.traits.getXPath;
+  },
+
   get canGetUsedFontFaces() {
     return this._target.client.traits.getUsedFontFaces;
   },
 
   get canPasteInnerOrAdjacentHTML() {
     return this._target.client.traits.pasteHTML;
   },
 
@@ -1241,16 +1245,25 @@ Inspector.prototype = {
       label: INSPECTOR_L10N.getStr("inspectorCopyCSSPath.label"),
       accesskey:
         INSPECTOR_L10N.getStr("inspectorCopyCSSPath.accesskey"),
       disabled: !isSelectionElement,
       hidden: !this.canGetCssPath,
       click: () => this.copyCssPath(),
     }));
     copySubmenu.append(new MenuItem({
+      id: "node-menu-copyxpath",
+      label: INSPECTOR_L10N.getStr("inspectorCopyXPath.label"),
+      accesskey:
+        INSPECTOR_L10N.getStr("inspectorCopyXPath.accesskey"),
+      disabled: !isSelectionElement,
+      hidden: !this.canGetXPath,
+      click: () => this.copyXPath(),
+    }));
+    copySubmenu.append(new MenuItem({
       id: "node-menu-copyimagedatauri",
       label: INSPECTOR_L10N.getStr("inspectorImageDataUri.label"),
       disabled: !isSelectionElement || !markupContainer ||
                 !markupContainer.isPreviewable(),
       click: () => this.copyImageDataUri(),
     }));
 
     return copySubmenu;
@@ -1798,16 +1811,30 @@ Inspector.prototype = {
 
     this.telemetry.toolOpened("copyfullcssselector");
     this.selection.nodeFront.getCssPath().then(path => {
       clipboardHelper.copyString(path);
     }).catch(e => console.error);
   },
 
   /**
+   * Copy the XPath of the selected Node to the clipboard.
+   */
+  copyXPath: function () {
+    if (!this.selection.isNode()) {
+      return;
+    }
+
+    this.telemetry.toolOpened("copyxpath");
+    this.selection.nodeFront.getXPath().then(path => {
+      clipboardHelper.copyString(path);
+    }).catch(e => console.error);
+  },
+
+  /**
    * Initiate gcli screenshot command on selected node.
    */
   screenshotNode: Task.async(function* () {
     const command = Services.prefs.getBoolPref("devtools.screenshot.clipboard.enabled") ?
       "screenshot --file --clipboard --selector" :
       "screenshot --file --selector";
 
     // Bug 1332936 - it's possible to call `screenshotNode` while the BoxModel highlighter
--- a/devtools/client/inspector/test/browser_inspector_menu-01-sensitivity.js
+++ b/devtools/client/inspector/test/browser_inspector_menu-01-sensitivity.js
@@ -22,16 +22,17 @@ const ACTIVE_ON_DOCTYPE_ITEMS = [
 ];
 
 const ALL_MENU_ITEMS = [
   "node-menu-edithtml",
   "node-menu-copyinner",
   "node-menu-copyouter",
   "node-menu-copyuniqueselector",
   "node-menu-copycsspath",
+  "node-menu-copyxpath",
   "node-menu-copyimagedatauri",
   "node-menu-delete",
   "node-menu-pseudo-hover",
   "node-menu-pseudo-active",
   "node-menu-pseudo-focus",
   "node-menu-scrollnodeintoview",
   "node-menu-screenshotnode",
   "node-menu-add-attribute",
--- a/devtools/client/inspector/test/browser_inspector_menu-02-copy-items.js
+++ b/devtools/client/inspector/test/browser_inspector_menu-02-copy-items.js
@@ -3,16 +3,18 @@
 http://creativecommons.org/publicdomain/zero/1.0/ */
 "use strict";
 
 // Test that the various copy items in the context menu works correctly.
 
 const TEST_URL = URL_ROOT + "doc_inspector_menu.html";
 const SELECTOR_UNIQUE = "devtools.copy.unique.css.selector.opened";
 const SELECTOR_FULL = "devtools.copy.full.css.selector.opened";
+const XPATH = "devtools.copy.xpath.opened";
+
 const COPY_ITEMS_TEST_DATA = [
   {
     desc: "copy inner html",
     id: "node-menu-copyinner",
     selector: "[data-id=\"copy\"]",
     text: "Paragraph for testing copy",
   },
   {
@@ -29,16 +31,22 @@ const COPY_ITEMS_TEST_DATA = [
   },
   {
     desc: "copy CSS path",
     id: "node-menu-copycsspath",
     selector: "[data-id=\"copy\"]",
     text: "html body div p",
   },
   {
+    desc: "copy XPath",
+    id: "node-menu-copyxpath",
+    selector: "[data-id=\"copy\"]",
+    text: "/html/body/div/p[1]",
+  },
+  {
     desc: "copy image data uri",
     id: "node-menu-copyimagedatauri",
     selector: "#copyimage",
     text: "data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAAEAAAABC" +
       "AAAAAA6fptVAAAACklEQVQYV2P4DwABAQEAWk1v8QAAAABJRU5ErkJggg==",
   },
 ];
 
@@ -68,20 +76,23 @@ function checkTelemetryResults(Telemetry
   for (let key in data) {
     if (key.toLowerCase() === key) {
       let pings = data[key].length;
 
       results.set(key, pings);
     }
   }
 
-  is(results.size, 2, "The correct number of scalars were logged");
+  is(results.size, 3, "The correct number of scalars were logged");
 
   let pings = checkPings(SELECTOR_UNIQUE, results);
   is(pings, 1, `${SELECTOR_UNIQUE} has just 1 ping`);
 
   pings = checkPings(SELECTOR_FULL, results);
   is(pings, 1, `${SELECTOR_FULL} has just 1 ping`);
+
+  pings = checkPings(XPATH, results);
+  is(pings, 1, `${XPATH} has just 1 ping`);
 }
 
 function checkPings(scalarId, results) {
   return results.get(scalarId);
 }
--- a/devtools/client/locales/en-US/inspector.properties
+++ b/devtools/client/locales/en-US/inspector.properties
@@ -168,16 +168,22 @@ inspectorCopyCSSSelector.label=CSS Selec
 inspectorCopyCSSSelector.accesskey=S
 
 # LOCALIZATION NOTE (inspectorCopyCSSPath.label): This is the label
 # shown in the inspector contextual-menu for the item that lets users copy
 # the full CSS path of the current node
 inspectorCopyCSSPath.label=CSS Path
 inspectorCopyCSSPath.accesskey=P
 
+# LOCALIZATION NOTE (inspectorCopyXPath.label): This is the label
+# shown in the inspector contextual-menu for the item that lets users copy
+# the XPath of the current node
+inspectorCopyXPath.label=XPath
+inspectorCopyXPath.accesskey=X
+
 # LOCALIZATION NOTE (inspectorPasteOuterHTML.label): This is the label shown
 # in the inspector contextual-menu for the item that lets users paste outer
 # HTML in the current node
 inspectorPasteOuterHTML.label=Outer HTML
 inspectorPasteOuterHTML.accesskey=O
 
 # LOCALIZATION NOTE (inspectorPasteInnerHTML.label): This is the label shown
 # in the inspector contextual-menu for the item that lets users paste inner
--- a/devtools/client/shared/telemetry.js
+++ b/devtools/client/shared/telemetry.js
@@ -141,16 +141,19 @@ Telemetry.prototype = {
       scalar: "devtools.toolbar.eyedropper.opened",
     },
     copyuniquecssselector: {
       scalar: "devtools.copy.unique.css.selector.opened",
     },
     copyfullcssselector: {
       scalar: "devtools.copy.full.css.selector.opened",
     },
+    copyxpath: {
+      scalar: "devtools.copy.xpath.opened",
+    },
     developertoolbar: {
       histogram: "DEVTOOLS_DEVELOPERTOOLBAR_OPENED_COUNT",
       timerHistogram: "DEVTOOLS_DEVELOPERTOOLBAR_TIME_ACTIVE_SECONDS"
     },
     aboutdebugging: {
       histogram: "DEVTOOLS_ABOUTDEBUGGING_OPENED_COUNT",
       timerHistogram: "DEVTOOLS_ABOUTDEBUGGING_TIME_ACTIVE_SECONDS"
     },
--- a/devtools/server/actors/inspector.js
+++ b/devtools/server/actors/inspector.js
@@ -154,16 +154,17 @@ loader.lazyGetter(this, "DOMParser", fun
 loader.lazyGetter(this, "eventListenerService", function () {
   return Cc["@mozilla.org/eventlistenerservice;1"]
            .getService(Ci.nsIEventListenerService);
 });
 
 loader.lazyRequireGetter(this, "CssLogic", "devtools/server/css-logic", true);
 loader.lazyRequireGetter(this, "findCssSelector", "devtools/shared/inspector/css-logic", true);
 loader.lazyRequireGetter(this, "getCssPath", "devtools/shared/inspector/css-logic", true);
+loader.lazyRequireGetter(this, "getXPath", "devtools/shared/inspector/css-logic", true);
 
 /**
  * We only send nodeValue up to a certain size by default.  This stuff
  * controls that size.
  */
 exports.DEFAULT_VALUE_SUMMARY_LENGTH = 50;
 var gValueSummaryLength = exports.DEFAULT_VALUE_SUMMARY_LENGTH;
 
@@ -661,16 +662,28 @@ var NodeActor = exports.NodeActor = prot
   getCssPath: function () {
     if (Cu.isDeadWrapper(this.rawNode)) {
       return "";
     }
     return getCssPath(this.rawNode);
   },
 
   /**
+   * Get the XPath for this node.
+   *
+   * @return {String} The XPath for finding this node on the page.
+   */
+  getXPath: function () {
+    if (Cu.isDeadWrapper(this.rawNode)) {
+      return "";
+    }
+    return getXPath(this.rawNode);
+  },
+
+  /**
    * Scroll the selected node into view.
    */
   scrollIntoView: function () {
     this.rawNode.scrollIntoView(true);
   },
 
   /**
    * Get the node's image data if any (for canvas and img nodes).
--- a/devtools/server/actors/root.js
+++ b/devtools/server/actors/root.js
@@ -147,16 +147,18 @@ RootActor.prototype = {
     selectorEditable: true,
     // Whether the page style actor implements the addNewRule method that
     // adds new rules to the page
     addNewRule: true,
     // Whether the dom node actor implements the getUniqueSelector method
     getUniqueSelector: true,
     // Whether the dom node actor implements the getCssPath method
     getCssPath: true,
+    // Whether the dom node actor implements the getXPath method
+    getXPath: true,
     // Whether the director scripts are supported
     directorScripts: true,
     // Whether the debugger server supports
     // blackboxing/pretty-printing (not supported in Fever Dream yet)
     noBlackBoxing: false,
     noPrettyPrinting: false,
     // Whether the page style actor implements the getUsedFontFaces method
     // that returns the font faces used on a node
--- a/devtools/shared/inspector/css-logic.js
+++ b/devtools/shared/inspector/css-logic.js
@@ -398,8 +398,68 @@ function getCssPath(ele) {
 
     paths.splice(0, 0, getElementSelector(ele));
     ele = ele.parentNode;
   }
 
   return paths.length ? paths.join(" ") : "";
 }
 exports.getCssPath = getCssPath;
+
+/**
+ * Get the xpath for a given element.
+ * @param {DomNode} ele
+ * @returns a string that can be used as an XPath to find the element uniquely.
+ */
+function getXPath(ele) {
+  ele = getRootBindingParent(ele);
+  const document = ele.ownerDocument;
+  if (!document || !document.contains(ele)) {
+    throw new Error("getXPath received element not inside document");
+  }
+
+  // Create a short XPath for elements with IDs.
+  if (ele.id) {
+    return `//*[@id="${ele.id}"]`;
+  }
+
+  // Otherwise walk the DOM up and create a part for each ancestor.
+  const parts = [];
+
+  // Use nodeName (instead of localName) so namespace prefix is included (if any).
+  while (ele && ele.nodeType === Node.ELEMENT_NODE) {
+    let nbOfPreviousSiblings = 0;
+    let hasNextSiblings = false;
+
+    // Count how many previous same-name siblings the element has.
+    let sibling = ele.previousSibling;
+    while (sibling) {
+      // Ignore document type declaration.
+      if (sibling.nodeType !== Node.DOCUMENT_TYPE_NODE &&
+          sibling.nodeName == ele.nodeName) {
+        nbOfPreviousSiblings++;
+      }
+
+      sibling = sibling.previousSibling;
+    }
+
+    // Check if the element has at least 1 next same-name sibling.
+    sibling = ele.nextSibling;
+    while (sibling) {
+      if (sibling.nodeName == ele.nodeName) {
+        hasNextSiblings = true;
+        break;
+      }
+      sibling = sibling.nextSibling;
+    }
+
+    const prefix = ele.prefix ? ele.prefix + ":" : "";
+    const nth = nbOfPreviousSiblings || hasNextSiblings
+                ? `[${nbOfPreviousSiblings + 1}]` : "";
+
+    parts.push(prefix + ele.localName + nth);
+
+    ele = ele.parentNode;
+  }
+
+  return parts.length ? "/" + parts.reverse().join("/") : "";
+}
+exports.getXPath = getXPath;
--- a/devtools/shared/specs/node.js
+++ b/devtools/shared/specs/node.js
@@ -38,16 +38,22 @@ const nodeSpec = generateActorSpec({
       }
     },
     getCssPath: {
       request: {},
       response: {
         value: RetVal("string")
       }
     },
+    getXPath: {
+      request: {},
+      response: {
+        value: RetVal("string")
+      }
+    },
     scrollIntoView: {
       request: {},
       response: {}
     },
     getImageData: {
       request: {maxDim: Arg(0, "nullable:number")},
       response: RetVal("imageData")
     },
--- a/devtools/shared/tests/mochitest/chrome.ini
+++ b/devtools/shared/tests/mochitest/chrome.ini
@@ -1,7 +1,8 @@
 [DEFAULT]
 tags = devtools
 skip-if = os == 'android'
 
 [test_css-logic-getCssPath.html]
+[test_css-logic-getXPath.html]
 [test_eventemitter_basic.html]
 skip-if = os == 'linux' && debug # Bug 1205739
new file mode 100644
--- /dev/null
+++ b/devtools/shared/tests/mochitest/test_css-logic-getXPath.html
@@ -0,0 +1,111 @@
+<!DOCTYPE HTML>
+<html>
+<!--
+https://bugzilla.mozilla.org/show_bug.cgi?id=987877
+-->
+<head>
+  <meta charset="utf-8">
+  <title>Test for Bug 987877</title>
+
+  <script type="application/javascript" src="chrome://mochikit/content/tests/SimpleTest/SimpleTest.js"></script>
+  <link rel="stylesheet" type="text/css" href="chrome://mochikit/content/tests/SimpleTest/test.css">
+  <script type="application/javascript">
+"use strict";
+
+const { utils: Cu } = Components;
+const { require } = Cu.import("resource://devtools/shared/Loader.jsm", {});
+const CssLogic = require("devtools/shared/inspector/css-logic");
+
+const _tests = [];
+function addTest(test) {
+  _tests.push(test);
+}
+
+function runNextTest() {
+  if (_tests.length == 0) {
+    SimpleTest.finish();
+    return;
+  }
+  _tests.shift()();
+}
+
+window.onload = function () {
+  SimpleTest.waitForExplicitFinish();
+  runNextTest();
+};
+
+addTest(function getXPathForUnattachedElement() {
+  let unattached = document.createElement("div");
+  unattached.id = "unattached";
+  try {
+    CssLogic.getXPath(unattached);
+    ok(false, "Unattached node did not throw");
+  } catch (e) {
+    ok(e, "Unattached node throws an exception");
+  }
+
+  let unattachedChild = document.createElement("div");
+  unattached.appendChild(unattachedChild);
+  try {
+    CssLogic.getXPath(unattachedChild);
+    ok(false, "Unattached child node did not throw");
+  } catch (e) {
+    ok(e, "Unattached child node throws an exception");
+  }
+
+  let unattachedBody = document.createElement("body");
+  try {
+    CssLogic.getXPath(unattachedBody);
+    ok(false, "Unattached body node did not throw");
+  } catch (e) {
+    ok(e, "Unattached body node throws an exception");
+  }
+
+  runNextTest();
+});
+
+addTest(function getXPath() {
+  let data = [{
+    // Target elements that have an ID get a short XPath.
+    selector: "#i-have-an-id",
+    path: "//*[@id=\"i-have-an-id\"]"
+  }, {
+    selector: "html",
+    path: "/html"
+  }, {
+    selector: "body",
+    path: "/html/body"
+  }, {
+    selector: "body > div:nth-child(2) > div > div:nth-child(4)",
+    path: "/html/body/div[2]/div/div[4]"
+  }, {
+    // XPath should support namespace.
+    selector: "namespace\\:body",
+    path: "/html/body/namespace:test/namespace:body"
+  }];
+
+  for (let {selector, path} of data) {
+    let node = document.querySelector(selector);
+    is(CssLogic.getXPath(node), path, `Full css path is correct for ${selector}`);
+  }
+
+  runNextTest();
+});
+  </script>
+</head>
+<body>
+  <div id="i-have-an-id">find me</div>
+  <div>
+    <div>
+      <div></div>
+      <div></div>
+      <div></div>
+      <div>me too!</div>
+    </div>
+  </div>
+  <namespace:test>
+    <namespace:header></namespace:header>
+    <namespace:body>and me</namespace:body>
+  </namespace:test>
+</body>
+</html>
--- a/toolkit/components/telemetry/Scalars.yaml
+++ b/toolkit/components/telemetry/Scalars.yaml
@@ -440,16 +440,29 @@ devtools.copy.full.css.selector:
     expires: "57"
     kind: uint
     notification_emails:
       - dev-developer-tools@lists.mozilla.org
     release_channel_collection: opt-out
     record_in_processes:
       - 'main'
 
+devtools.copy.xpath:
+  opened:
+    bug_numbers:
+      - 987877
+    description: Number of times the DevTools copy XPath has been used.
+    expires: "58"
+    kind: uint
+    notification_emails:
+      - dev-developer-tools@lists.mozilla.org
+    release_channel_collection: opt-out
+    record_in_processes:
+      - 'main'
+
 navigator.storage:
   estimate_count:
     bug_numbers:
       - 1359708
     description: >
       Number of times navigator.storage.estimate has been used.
     expires: "60"
     kind: uint