Bug 987877 - Add Copy XPath menu item to the inspector; r=miker draft
authorPatrick Brosset <pbrosset@mozilla.com>
Mon, 12 Jun 2017 16:25:48 +0200
changeset 592658 c426b97a789847ba8a6c1a7d3f8cc81cd20a2fdc
parent 592639 b99b018d5f005aca15fe4a57418b96b6826fdcd7
child 632889 4d8131bfa92812b18b54e3d8609f5644946314a6
push id63461
push userbmo:pbrosset@mozilla.com
push dateMon, 12 Jun 2017 15:33:06 +0000
reviewersmiker
bugs987877
milestone56.0a1
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