Bug 1565715 - Stop preview highlight at non-element nodes.

By tweaking the while loop in addHighlightToTargetSiblings, the preview highlight will not continue beyond a non-text node. (Example in the case of a semi colon or a period.

Differential Revision: https://phabricator.services.mozilla.com/D44275

--HG--
extra : moz-landing-system : lando

devtools/client/debugger/src/components/Editor/Preview/Popup.js

@@ -222,10 +222,12 @@ export class Popup extends Component<Props> {
   }
 }
 
-function addHighlightToTargetSiblings(target: Element, props: Object) {
-  // Look at target's pervious and next token siblings.
-  // If they are the same token type, and are also found in the preview expression,
-  // add the highlight class to them as well.
+export function addHighlightToTargetSiblings(target: Element, props: Object) {
+  // This function searches for related tokens that should also be highlighted when previewed.
+  // Here is the process:
+  // It conducts a search on the target's next siblings and then another search for the previous siblings.
+  // If a sibling is not an element node (nodeType === 1), the highlight is not added and the search is short-circuited.
+  // If the element sibling is the same token type as the target, and is also found in the preview expression, the highlight class is added.
 
   const tokenType = target.classList.item(0);
   const previewExpression = props.preview.expression;
@@ -235,28 +237,48 @@ function addHighlightToTargetSiblings(target: Element, props: Object) {
     previewExpression &&
     target.innerHTML !== previewExpression
   ) {
-    let nextSibling = target.nextElementSibling;
-    while (
-      nextSibling &&
-      nextSibling.className.includes(tokenType) &&
-      previewExpression.includes(nextSibling.innerHTML)
+    let nextSibling = target.nextSibling;
+    let nextElementSibling = target.nextElementSibling;
+    // Note: Declaring previous/next ELEMENT siblings as well because
+    // properties like innerHTML can't be checked on nextSibling
+    // without creating a flow error even if the node is an element type.
+
+    while (nextSibling && nextElementSibling && nextSibling.nodeType === 1) {
+      if (
+        nextElementSibling.className.includes(tokenType) &&
+        previewExpression.includes(nextElementSibling.innerHTML)
       ) {
-      nextSibling.classList.add("preview-token");
-      nextSibling = nextSibling.nextElementSibling;
+        // All checks passed, add highlight and continue the search.
+        nextElementSibling.classList.add("preview-token");
+
+        nextSibling = nextSibling.nextSibling;
+        nextElementSibling = nextElementSibling.nextElementSibling;
       }
-    let previousSibling = target.previousElementSibling;
+    }
+
+    let previousSibling = target.previousSibling;
+    let previousElementSibling = target.previousElementSibling;
+
     while (
       previousSibling &&
-      previousSibling.className.includes(tokenType) &&
-      previewExpression.includes(previousSibling.innerHTML)
+      previousElementSibling &&
+      previousSibling.nodeType === 1
     ) {
-      previousSibling.classList.add("preview-token");
-      previousSibling = previousSibling.previousElementSibling;
+      if (
+        previousElementSibling.className.includes(tokenType) &&
+        previewExpression.includes(previousElementSibling.innerHTML)
+      ) {
+        // All checks passed, add highlight and continue the search.
+        previousElementSibling.classList.add("preview-token");
+
+        previousSibling = previousSibling.previousSibling;
+        previousElementSibling = previousElementSibling.previousElementSibling;
+      }
     }
   }
 }
 
-function removeHighlightForTargetSiblings(target: Element) {
+export function removeHighlightForTargetSiblings(target: Element) {
   // Look at target's previous and next token siblings.
   // If they also have the highlight class 'preview-token',
   // remove that class.

devtools/client/debugger/src/components/Editor/Preview/tests/Popup.spec.js

@@ -0,0 +1,109 @@
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at <http://mozilla.org/MPL/2.0/>. */
+
+// @flow
+
+import {
+  addHighlightToTargetSiblings,
+  removeHighlightForTargetSiblings,
+} from "../Popup";
+
+describe("addHighlightToTargetSiblings", () => {
+  it("should add preview highlight class to related target siblings", async () => {
+    const div = document.createElement("div");
+    const divChildren = ["a", "divided", "token"];
+    divChildren.forEach(function(span) {
+      const child = document.createElement("span");
+      const text = document.createTextNode(span);
+      child.appendChild(text);
+      child.classList.add("cm-property");
+      div.appendChild(child);
+    });
+
+    const target = div.children[1];
+    const props = {
+      preview: {
+        expression: "adividedtoken",
+      },
+    };
+
+    addHighlightToTargetSiblings(target, props);
+
+    const previous = target.previousElementSibling;
+    if (previous && previous.className) {
+      expect(previous.className.includes("preview-token")).toEqual(true);
+    }
+
+    const next = target.nextElementSibling;
+    if (next && next.className) {
+      expect(next.className.includes("preview-token")).toEqual(true);
+    }
+  });
+
+  it("should not add preview highlight class to target's related siblings after non-element nodes", () => {
+    const div = document.createElement("div");
+
+    const elementBeforePeriod = document.createElement("span");
+    elementBeforePeriod.innerHTML = "object";
+    elementBeforePeriod.classList.add("cm-property");
+    div.appendChild(elementBeforePeriod);
+
+    const period = document.createTextNode(".");
+    div.appendChild(period);
+
+    const target = document.createElement("span");
+    target.innerHTML = "property";
+    target.classList.add("cm-property");
+    div.appendChild(target);
+
+    const anotherPeriod = document.createTextNode(".");
+    div.appendChild(anotherPeriod);
+
+    const elementAfterPeriod = document.createElement("span");
+    elementAfterPeriod.innerHTML = "anotherProperty";
+    elementAfterPeriod.classList.add("cm-property");
+    div.appendChild(elementAfterPeriod);
+
+    const props = {
+      preview: {
+        expression: "object.property.anotherproperty",
+      },
+    };
+    addHighlightToTargetSiblings(target, props);
+
+    expect(elementBeforePeriod.className.includes("preview-token")).toEqual(
+      false
+    );
+    expect(elementAfterPeriod.className.includes("preview-token")).toEqual(
+      false
+    );
+  });
+});
+
+describe("removeHighlightForTargetSiblings", () => {
+  it("should remove preview highlight class from target's related siblings", async () => {
+    const div = document.createElement("div");
+    const divChildren = ["a", "divided", "token"];
+    divChildren.forEach(function(span) {
+      const child = document.createElement("span");
+      const text = document.createTextNode(span);
+      child.appendChild(text);
+      child.classList.add("preview-token");
+      div.appendChild(child);
+    });
+    const target = div.children[1];
+
+    removeHighlightForTargetSiblings(target);
+
+    const previous = target.previousElementSibling;
+    if (previous && previous.className) {
+      expect(previous.className.includes("preview-token")).toEqual(false);
+    }
+
+    const next = target.nextElementSibling;
+    if (next && next.className) {
+      expect(next.className.includes("preview-token")).toEqual(false);
+    }
+  });
+});