WebUI Polymer2: Fix highlight removal (Settings and Print Preview)

Since /deep/ is deprecated in shadow DOM v1, track highlights and
search bubbles as they are added to avoid searching recursively to
remove them later.

Bug: 852098
Cq-Include-Trybots: luci.chromium.try:closure_compilation
Change-Id: Iac61a1f340ae4029589556c3ad09e567126a1639
Reviewed-on: https://chromium-review.googlesource.com/1100102
Commit-Queue: Rebekah Potter <rbpotter@chromium.org>
Reviewed-by: Demetrios Papadopoulos <dpapad@chromium.org>
Cr-Commit-Position: refs/heads/master@{#568108}

chrome/browser/resources/print_preview/new/advanced_settings_dialog.js

@@ -25,6 +25,12 @@ Polymer({
     },
   },
 
+  /** @private {!Array<Node>} */
+  highlights_: [],
+
+  /** @private {!Array<Node>} */
+  bubbles_: [],
+
   /** @private {!print_preview.PrintSettingsUiMetricsContext} */
   metrics_: new print_preview.PrintSettingsUiMetricsContext(),
 
@@ -41,6 +47,12 @@ Polymer({
    * @private
    */
   computeHasMatching_: function() {
+    cr.search_highlight_utils.removeHighlights(this.highlights_);
+    for (let bubble of this.bubbles_)
+      bubble.remove();
+    this.highlights_ = [];
+    this.bubbles_ = [];
+
     const listItems = this.shadowRoot.querySelectorAll(
         'print-preview-advanced-settings-item');
     let hasMatch = false;
@@ -48,7 +60,9 @@ Polymer({
       const matches = item.hasMatch(this.searchQuery_);
       item.hidden = !matches;
       hasMatch = hasMatch || matches;
-      item.updateHighlighting(this.searchQuery_);
+      const result = item.updateHighlighting(this.searchQuery_);
+      this.highlights_.push.apply(this.highlights_, result.highlights);
+      this.bubbles_.push.apply(this.bubbles_, result.bubbles);
     });
     return hasMatch;
   },

chrome/browser/resources/print_preview/new/advanced_settings_item.js

@@ -22,9 +22,6 @@ Polymer({
     'updateFromSettings_(capability, settings.vendorItems.value)',
   ],
 
-  /** @private {boolean} */
-  highlighted_: false,
-
   /** @private */
   updateFromSettings_: function() {
     const settings = this.getSetting('vendorItems').value;
@@ -152,11 +149,10 @@ Polymer({
 
   /**
    * @param {?RegExp} query The current search query.
-   * @return {boolean} Whether the current query is a match for this item.
+   * @return {!print_preview.HighlightResults} The highlight wrappers and
+   *     search bubbles that were created.
    */
   updateHighlighting: function(query) {
-    this.highlighted_ =
-        print_preview.updateHighlights(this, query, this.highlighted_);
-    return this.highlighted_ || !query;
+    return print_preview.updateHighlights(this, query);
   },
 });

chrome/browser/resources/print_preview/new/destination_list.js

@@ -60,6 +60,9 @@ Polymer({
   /** @private {boolean} */
   newDestinations_: false,
 
+  /** @private {!Array<!Node>} */
+  highlights_: [],
+
   /**
    * @param {!Array<!print_preview.Destination>} current
    * @param {?Array<!print_preview.Destination>} previous
@@ -86,6 +89,9 @@ Polymer({
     if (!this.destinations)
       return;
 
+    cr.search_highlight_utils.removeHighlights(this.highlights_);
+    this.highlights_ = [];
+
     const listItems =
         this.shadowRoot.querySelectorAll('print-preview-destination-list-item');
 
@@ -95,7 +101,7 @@ Polymer({
           !!this.searchQuery && !item.destination.matches(this.searchQuery);
       if (!item.hidden) {
         matchCount++;
-        item.update();
+        this.highlights_.push.apply(this.highlights_, item.update().highlights);
       }
     });
 

chrome/browser/resources/print_preview/new/destination_list_item.js

@@ -56,9 +56,6 @@ Polymer({
         'destination.isExtension)',
   ],
 
-  /** @private {boolean} */
-  highlighted_: false,
-
   /** @private */
   onDestinationPropertiesChange_: function() {
     this.title = this.destination.displayName;
@@ -113,9 +110,13 @@ Polymer({
   },
   // </if>
 
+  /**
+   * @return {!print_preview.HighlightResults} The highlight wrappers and
+   *     search bubbles that were created.
+   */
   update: function() {
     this.updateSearchHint_();
-    this.updateHighlighting_();
+    return this.updateHighlighting_();
   },
 
   /** @private */
@@ -127,9 +128,12 @@ Polymer({
             .join(' ');
   },
 
-  /** @private */
+  /**
+   * @return {!print_preview.HighlightResults} The highlight wrappers and
+   *     search bubbles that were created.
+   * @private
+   */
   updateHighlighting_: function() {
-    this.highlighted_ = print_preview.updateHighlights(
-        this, this.searchQuery, this.highlighted_);
+    return print_preview.updateHighlights(this, this.searchQuery);
   },
 });

chrome/browser/resources/print_preview/new/highlight_utils.js

@@ -2,6 +2,16 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+cr.exportPath('print_preview');
+
+/**
+ * @typedef {{
+ *   highlights: !Array<!Node>,
+ *   bubbles: !Array<!Node>
+ * }}
+ */
+print_preview.HighlightResults;
+
 cr.define('print_preview', function() {
   'use strict';
 
@@ -9,20 +19,14 @@ cr.define('print_preview', function() {
    * @param {!HTMLElement} element The element to update. Element should have a
    *     shadow root.
    * @param {?RegExp} query The current search query
-   * @param {boolean} wasHighlighted Whether the element was previously
-   *     highlighted.
-   * @return {boolean} Whether the element is highlighted after the update.
+   * @return {!print_preview.HighlightResults} The highlight wrappers and
+   *     search bubbles that were created.
    */
-  function updateHighlights(element, query, wasHighlighted) {
-    if (wasHighlighted) {
-      cr.search_highlight_utils.findAndRemoveHighlights(element);
-      cr.search_highlight_utils.findAndRemoveBubbles(element);
-    }
-
+  function updateHighlights(element, query) {
+    const result = {highlights: [], bubbles: []};
     if (!query)
-      return false;
+      return result;
 
-    let isHighlighted = false;
     element.shadowRoot.querySelectorAll('.searchable').forEach(childElement => {
       childElement.childNodes.forEach(node => {
         if (node.nodeType != Node.TEXT_NODE)
@@ -33,25 +37,27 @@ cr.define('print_preview', function() {
           return;
 
         if (query.test(textContent)) {
-          isHighlighted = true;
           // Don't highlight <select> nodes, yellow rectangles can't be
           // displayed within an <option>.
           if (node.parentNode.nodeName != 'OPTION') {
-            cr.search_highlight_utils.highlight(node, textContent.split(query));
+            result.highlights.push(cr.search_highlight_utils.highlight(
+                node, textContent.split(query)));
           } else {
             const selectNode = node.parentNode.parentNode;
             // The bubble should be parented by the select node's parent.
             // Note: The bubble's ::after element, a yellow arrow, will not
             // appear correctly in print preview without SPv175 enabled. See
             // https://crbug.com/817058.
-            cr.search_highlight_utils.highlightControlWithBubble(
+            const bubble = cr.search_highlight_utils.highlightControlWithBubble(
                 /** @type {!HTMLElement} */ (assert(selectNode.parentNode)),
                 textContent.match(query)[0]);
+            if (bubble)
+              result.bubbles.push(bubble);
           }
         }
       });
     });
-    return isHighlighted;
+    return result;
   }
 
   return {

chrome/browser/resources/settings/search_settings.js

@@ -47,19 +47,6 @@ cr.define('settings', function() {
     'TEMPLATE',
   ]);
 
-  /**
-   * Finds all previous highlighted nodes under |node| (both within self and
-   * children's Shadow DOM) and replaces the highlights (yellow rectangle and
-   * search bubbles) with the original text node.
-   * TODO(dpapad): Consider making this a private method of TopLevelSearchTask.
-   * @param {!Node} node
-   * @private
-   */
-  function findAndRemoveHighlights_(node) {
-    cr.search_highlight_utils.findAndRemoveHighlights(node);
-    cr.search_highlight_utils.findAndRemoveBubbles(node);
-  }
-
   /**
    * Traverses the entire DOM (including Shadow DOM), finds text nodes that
    * match the given regular expression and applies the highlight UI. It also
@@ -72,6 +59,8 @@ cr.define('settings', function() {
    */
   function findAndHighlightMatches_(request, root) {
     let foundMatches = false;
+    let highlights = [];
+    let bubbles = [];
     function doSearch(node) {
       if (node.nodeName == 'TEMPLATE' && node.hasAttribute('route-path') &&
           !node.if && !node.hasAttribute(SKIP_SEARCH_CSS_ATTRIBUTE)) {
@@ -97,7 +86,9 @@ cr.define('settings', function() {
 
         if (request.regExp.test(textContent)) {
           foundMatches = true;
-          revealParentSection_(node, request.rawQuery_);
+          let bubble = revealParentSection_(node, request.rawQuery_);
+          if (bubble)
+            bubbles.push(bubble);
 
           // Don't highlight <select> nodes, yellow rectangles can't be
           // displayed within an <option>.
@@ -105,8 +96,8 @@ cr.define('settings', function() {
           // instead.
           if (node.parentNode.nodeName != 'OPTION') {
             request.addTextObserver(node);
-            cr.search_highlight_utils.highlight(
-                node, textContent.split(request.regExp));
+            highlights.push(cr.search_highlight_utils.highlight(
+                node, textContent.split(request.regExp)));
           }
         }
         // Returning early since TEXT_NODE nodes never have children.
@@ -128,6 +119,7 @@ cr.define('settings', function() {
     }
 
     doSearch(root);
+    request.addHighlightsAndBubbles(highlights, bubbles);
     return foundMatches;
   }
 
@@ -135,6 +127,8 @@ cr.define('settings', function() {
    * Finds and makes visible the <settings-section> parent of |node|.
    * @param {!Node} node
    * @param {string} rawQuery
+   * @return {?Node} The search bubble created while revealing the section, if
+   *     any.
    * @private
    */
   function revealParentSection_(node, rawQuery) {
@@ -159,9 +153,10 @@ cr.define('settings', function() {
     // Need to add the search bubble after the parent SETTINGS-SECTION has
     // become visible, otherwise |offsetWidth| returns zero.
     if (associatedControl) {
-      cr.search_highlight_utils.highlightControlWithBubble(
+      return cr.search_highlight_utils.highlightControlWithBubble(
           associatedControl, rawQuery);
     }
+    return null;
   }
 
   /** @abstract */
@@ -251,8 +246,6 @@ cr.define('settings', function() {
 
     /** @override */
     exec() {
-      findAndRemoveHighlights_(this.node);
-
       const shouldSearch = this.request.regExp !== null;
       this.setSectionsVisibility_(!shouldSearch);
       if (shouldSearch) {
@@ -403,6 +396,21 @@ cr.define('settings', function() {
 
       /** @private {!Set<!MutationObserver>} */
       this.textObservers_ = new Set();
+
+      /** @private {!Array<!Node>} */
+      this.highlights_ = [];
+
+      /** @private {!Array<!Node>} */
+      this.bubbles_ = [];
+    }
+
+    /**
+     * @param {!Array<!Node>} highlights The highlight wrappers to add
+     * @param {!Array<!Node>} bubbles The search bubbles to add.
+     */
+    addHighlightsAndBubbles(highlights, bubbles) {
+      this.highlights_.push.apply(this.highlights_, highlights);
+      this.bubbles_.push.apply(this.bubbles_, bubbles);
     }
 
     removeAllTextObservers() {
@@ -412,6 +420,14 @@ cr.define('settings', function() {
       this.textObservers_.clear();
     }
 
+    removeAllHighlightsAndBubbles() {
+      cr.search_highlight_utils.removeHighlights(this.highlights_);
+      for (let bubble of this.bubbles_)
+        bubble.remove();
+      this.highlights_ = [];
+      this.bubbles_ = [];
+    }
+
     /** @param {!Node} textNode */
     addTextObserver(textNode) {
       const originalParentNode = /** @type {!Node} */ (textNode.parentNode);
@@ -510,12 +526,14 @@ cr.define('settings', function() {
       if (text != this.lastSearchedText_) {
         this.activeRequests_.forEach(function(request) {
           request.removeAllTextObservers();
+          request.removeAllHighlightsAndBubbles();
           request.canceled = true;
           request.resolver.resolve(request);
         });
         this.activeRequests_.clear();
         this.completedRequests_.forEach(request => {
           request.removeAllTextObservers();
+          request.removeAllHighlightsAndBubbles();
         });
         this.completedRequests_.clear();
       }

ui/webui/resources/js/search_highlight_utils.js

@@ -15,6 +15,36 @@ cr.define('cr.search_highlight_utils', function() {
   /** @type {string} */
   const SEARCH_BUBBLE_CSS_CLASS = 'search-bubble';
 
+  /**
+   * Replaces the the highlight wrappers given in |wrappers| with the original
+   * search nodes.
+   * @param {!Array<!Node>} wrappers
+   */
+  function removeHighlights(wrappers) {
+    for (let wrapper of wrappers) {
+      // If wrapper is already removed, do nothing.
+      if (!wrapper.parentElement)
+        continue;
+
+      const textNode =
+          wrapper.querySelector(`.${ORIGINAL_CONTENT_CSS_CLASS}`).firstChild;
+      wrapper.parentElement.replaceChild(textNode, wrapper);
+    }
+  }
+
+  /**
+   * Finds all previous highlighted nodes under |node| and replaces the
+   * highlights (yellow rectangles) with the original search node. Searches only
+   * within the same shadowRoot and assumes that only one highlight wrapper
+   * exists under |node|.
+   * @param {!Node} node
+   */
+  function findAndRemoveHighlights(node) {
+    const wrappers = Array.from(node.querySelectorAll(`.${WRAPPER_CSS_CLASS}`));
+    assert(wrappers.length == 1);
+    removeHighlights(wrappers);
+  }
+
   /**
    * Applies the highlight UI (yellow rectangle) around all matches in |node|.
    * @param {!Node} node The text node to be highlighted. |node| ends up
@@ -24,6 +54,7 @@ cr.define('cr.search_highlight_utils', function() {
    *     odd indices hold the text to be highlighted. For example:
    *     const r = new RegExp('(foo)', 'i');
    *     'barfoobar foo bar'.split(r) => ['bar', 'foo', 'bar ', 'foo', ' bar']
+   * @return {!Node} The new highlight wrapper.
    */
   function highlight(node, tokens) {
     const wrapper = document.createElement('span');
@@ -52,37 +83,7 @@ cr.define('cr.search_highlight_utils', function() {
         wrapper.appendChild(hitSpan);
       }
     }
-  }
-
-  /**
-   * Finds all previous highlighted nodes under |node| (both within self and
-   * children's Shadow DOM) and replaces the highlights (yellow rectangles)
-   * with the original search node.
-   * @param {!Node} node
-   * @private
-   */
-  function findAndRemoveHighlights(node) {
-    const wrappers = node.querySelectorAll(`* /deep/ .${WRAPPER_CSS_CLASS}`);
-
-    for (let i = 0; i < wrappers.length; i++) {
-      const wrapper = wrappers[i];
-      const textNode =
-          wrapper.querySelector(`.${ORIGINAL_CONTENT_CSS_CLASS}`).firstChild;
-      wrapper.parentElement.replaceChild(textNode, wrapper);
-    }
-  }
-
-  /**
-   * Finds and removes all previously created yellow search bubbles under
-   * |node| (both within self and children's Shadow DOM).
-   * @param {!Node} node
-   * @private
-   */
-  function findAndRemoveBubbles(node) {
-    const searchBubbles =
-        node.querySelectorAll(`* /deep/ .${SEARCH_BUBBLE_CSS_CLASS}`);
-    for (let bubble of searchBubbles)
-      bubble.remove();
+    return wrapper;
   }
 
   /**
@@ -90,6 +91,8 @@ cr.define('cr.search_highlight_utils', function() {
    * should already be visible or the bubble will render incorrectly.
    * @param {!HTMLElement} element The element to be highlighted.
    * @param {string} rawQuery The search query.
+   * @return {?Node} The search bubble that was added, or null if no new bubble
+   *     was added.
    * @private
    */
   function highlightControlWithBubble(element, rawQuery) {
@@ -97,7 +100,7 @@ cr.define('cr.search_highlight_utils', function() {
     // If the element has already been highlighted, there is no need to do
     // anything.
     if (searchBubble)
-      return;
+      return null;
 
     searchBubble = document.createElement('div');
     searchBubble.classList.add(SEARCH_BUBBLE_CSS_CLASS);
@@ -119,12 +122,13 @@ cr.define('cr.search_highlight_utils', function() {
       innards.classList.toggle('above');
       updatePosition();
     });
+    return searchBubble;
   }
 
   return {
+    removeHighlights: removeHighlights,
+    findAndRemoveHighlights: findAndRemoveHighlights,
     highlight: highlight,
     highlightControlWithBubble: highlightControlWithBubble,
-    findAndRemoveBubbles: findAndRemoveBubbles,
-    findAndRemoveHighlights: findAndRemoveHighlights,
   };
 });