Bug 1078374 - Return empty string in getPath/Selector methods for nodes outside document;r=bgrins draft
authorJulian Descottes <jdescottes@mozilla.com>
Sat, 23 Jun 2018 08:29:16 +0200
changeset 810773 be16034feeac2868938bc9a88ec30162f4d99433
parent 810772 af386c52748c55d518445c0175ac129c101a63da
child 810774 615e49721e6e7b527255d3dde910de931d33ccae
push id114098
push userjdescottes@mozilla.com
push dateTue, 26 Jun 2018 13:21:46 +0000
reviewersbgrins
bugs1078374
milestone62.0a1
Bug 1078374 - Return empty string in getPath/Selector methods for nodes outside document;r=bgrins MozReview-Commit-ID: 5hxtjz23vXh
devtools/shared/inspector/css-logic.js
devtools/shared/tests/mochitest/test_css-logic-getCssPath.html
devtools/shared/tests/mochitest/test_css-logic-getXPath.html
toolkit/modules/css-selector.js
toolkit/modules/tests/chrome/test_findCssSelector.html
--- a/devtools/shared/inspector/css-logic.js
+++ b/devtools/shared/inspector/css-logic.js
@@ -373,17 +373,18 @@ exports.findCssSelector = findCssSelecto
  * @returns a string that can be used as a CSS selector for the element. It might not
  * match the element uniquely. It does however, represent the full path from the root
  * node to the element.
  */
 function getCssPath(ele) {
   ele = getRootBindingParent(ele);
   const document = ele.ownerDocument;
   if (!document || !document.contains(ele)) {
-    throw new Error("getCssPath received element not inside document");
+    // getCssPath received element not inside document.
+    return "";
   }
 
   const getElementSelector = element => {
     if (!element.localName) {
       return "";
     }
 
     let label = element.nodeName == element.nodeName.toUpperCase()
@@ -422,17 +423,18 @@ 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");
+    // getXPath received element not inside document.
+    return "";
   }
 
   // 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.
--- a/devtools/shared/tests/mochitest/test_css-logic-getCssPath.html
+++ b/devtools/shared/tests/mochitest/test_css-logic-getCssPath.html
@@ -30,39 +30,24 @@ function runNextTest() {
 window.onload = function() {
   SimpleTest.waitForExplicitFinish();
   runNextTest();
 }
 
 addTest(function getCssPathForUnattachedElement() {
   var unattached = document.createElement("div");
   unattached.id = "unattached";
-  try {
-    CssLogic.getCssPath(unattached);
-    ok(false, "Unattached node did not throw")
-  } catch(e) {
-    ok(e, "Unattached node throws an exception");
-  }
+  is(CssLogic.getCssPath(unattached), "", "Unattached node returns empty string");
 
   var unattachedChild = document.createElement("div");
   unattached.appendChild(unattachedChild);
-  try {
-    CssLogic.getCssPath(unattachedChild);
-    ok(false, "Unattached child node did not throw")
-  } catch(e) {
-    ok(e, "Unattached child node throws an exception");
-  }
+  is(CssLogic.getCssPath(unattachedChild), "", "Unattached child returns empty string");
 
   var unattachedBody = document.createElement("body");
-  try {
-    CssLogic.getCssPath(unattachedBody);
-    ok(false, "Unattached body node did not throw")
-  } catch(e) {
-    ok(e, "Unattached body node throws an exception");
-  }
+  is(CssLogic.getCssPath(unattachedBody), "", "Unattached body returns empty string");
 
   runNextTest();
 });
 
 addTest(function cssPathHasOneStepForEachAncestor() {
   for (let el of [...document.querySelectorAll('*')]) {
     let splitPath = CssLogic.getCssPath(el).split(" ");
 
--- a/devtools/shared/tests/mochitest/test_css-logic-getXPath.html
+++ b/devtools/shared/tests/mochitest/test_css-logic-getXPath.html
@@ -31,39 +31,24 @@ function runNextTest() {
 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");
-  }
+  is(CssLogic.getXPath(unattached), "", "Unattached node returns empty string");
 
   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");
-  }
+  is(CssLogic.getXPath(unattachedChild), "", "Unattached child returns empty string");
 
   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");
-  }
+  is(CssLogic.getXPath(unattachedBody), "", "Unattached body returns empty string");
 
   runNextTest();
 });
 
 addTest(function getXPath() {
   let data = [{
     // Target elements that have an ID get a short XPath.
     selector: "#i-have-an-id",
--- a/toolkit/modules/css-selector.js
+++ b/toolkit/modules/css-selector.js
@@ -48,17 +48,18 @@ function positionInNodeList(element, nod
  * Find a unique CSS selector for a given element
  * @returns a string such that ele.ownerDocument.querySelector(reply) === ele
  * and ele.ownerDocument.querySelectorAll(reply).length === 1
  */
 const findCssSelector = function(ele) {
   ele = getRootBindingParent(ele);
   let document = ele.ownerDocument;
   if (!document || !document.contains(ele)) {
-    throw new Error("findCssSelector received element not inside document");
+    // findCssSelector received element not inside document.
+    return "";
   }
 
   let cssEscape = ele.ownerGlobal.CSS.escape;
 
   // document.querySelectorAll("#id") returns multiple if elements share an ID
   if (ele.id &&
       document.querySelectorAll("#" + cssEscape(ele.id)).length === 1) {
     return "#" + cssEscape(ele.id);
--- a/toolkit/modules/tests/chrome/test_findCssSelector.html
+++ b/toolkit/modules/tests/chrome/test_findCssSelector.html
@@ -44,39 +44,24 @@ addTest(function findAllCssSelectors() {
 
   runNextTest();
 });
 
 addTest(function findCssSelectorNotContainedInDocument() {
 
   var unattached = document.createElement("div");
   unattached.id = "unattached";
-  try {
-    findCssSelector(unattached);
-    ok(false, "Unattached node did not throw");
-  } catch (e) {
-    ok(e, "Unattached node throws an exception");
-  }
+  is(findCssSelector(unattached), "", "Unattached node returns empty string");
 
   var unattachedChild = document.createElement("div");
   unattached.appendChild(unattachedChild);
-  try {
-    findCssSelector(unattachedChild);
-    ok(false, "Unattached child node did not throw");
-  } catch (e) {
-    ok(e, "Unattached child node throws an exception");
-  }
+  is(findCssSelector(unattachedChild), "", "Unattached child returns empty string");
 
   var unattachedBody = document.createElement("body");
-  try {
-    findCssSelector(unattachedBody);
-    ok(false, "Unattached body node did not throw");
-  } catch (e) {
-    ok(e, "Unattached body node throws an exception");
-  }
+  is(findCssSelector(unattachedBody), "", "Unattached body returns empty string");
 
   runNextTest();
 });
 
 addTest(function findCssSelectorBasic() {
 
   let data = [
     "#one",