Remove util fn addElementsToPage, and rename refactors

This utils function wasn't pulling its weight.

Author: philc

content_scripts/link_hints.js

@@ -361,7 +361,7 @@ class LinkHintsMode {
     // We need documentElement to be ready in order to append links.
     if (!document.documentElement) return;
 
-    this.hintMarkerContainingDiv = null;
+    this.containerEl = null;
     // Function that does the appropriate action on the selected link.
     this.linkActivator = undefined;
     // The link-hints "mode" (in the key-handler, indicator sense).
@@ -407,20 +407,28 @@ class LinkHintsMode {
   }
 
   renderHints() {
+    if (this.containerEl == null) {
+      const div = DomUtils.createElement("div");
+      div.id = "vimiumHintMarkerContainer";
+      div.className = "vimiumReset";
+      this.containerEl = div;
+      document.documentElement.appendChild(div);
+    }
+
     // Append these markers as top level children instead of as child nodes to the link itself,
     // because some clickable elements cannot contain children, e.g. submit buttons.
-    this.hintMarkerContainingDiv = DomUtils.addElementsToPage(
-      this.hintMarkers.filter((m) => m.isLocalMarker()).map((m) => m.element),
-      { id: "vimiumHintMarkerContainer", className: "vimiumReset" }
-    );
+    const markerEls = this.hintMarkers.filter((m) => m.isLocalMarker()).map((m) => m.element);
+    for (const el of markerEls) {
+      this.containerEl.appendChild(el);
+    }
 
     // TODO(philc): 2024-03-27 Remove this hasPopoverSupport check once Firefox has popover support.
     // Also move this CSS into vimium.css.
-    const hasPopoverSupport = this.hintMarkerContainingDiv.showPopover != null;
+    const hasPopoverSupport = this.containerEl.showPopover != null;
     if (hasPopoverSupport) {
-      this.hintMarkerContainingDiv.popover = "manual";
-      this.hintMarkerContainingDiv.showPopover();
-      Object.assign(this.hintMarkerContainingDiv.style, {
+      this.containerEl.popover = "manual";
+      this.containerEl.showPopover();
+      Object.assign(this.containerEl.style, {
         top: 0,
         left: 0,
         position: "absolute",
@@ -762,10 +770,10 @@ class LinkHintsMode {
   }
 
   removeHintMarkers() {
-    if (this.hintMarkerContainingDiv) {
-      DomUtils.removeElement(this.hintMarkerContainingDiv);
+    if (this.containerEl) {
+      DomUtils.removeElement(this.containerEl);
     }
-    this.hintMarkerContainingDiv = null;
+    this.containerEl = null;
   }
 }
 

content_scripts/mode_normal.js

@@ -509,10 +509,14 @@ class FocusSelector extends Mode {
       },
     });
 
-    this.hintContainingDiv = DomUtils.addElementsToPage(hints, {
-      id: "vimiumInputMarkerContainer",
-      className: "vimiumReset",
-    });
+    const div = DomUtils.createElement("div");
+    div.id = "vimiumInputMarkerContainer";
+    div.className = "vimiumReset";
+    for (const el of hints) {
+      div.appendChild(el);
+    }
+    this.hintContainerEl = div;
+    document.documentElement.appendChild(div);
 
     DomUtils.simulateSelect(visibleInputs[selectedInputIndex].element);
     if (visibleInputs.length === 1) {
@@ -525,7 +529,7 @@ class FocusSelector extends Mode {
 
   exit() {
     super.exit();
-    DomUtils.removeElement(this.hintContainingDiv);
+    DomUtils.removeElement(this.hintContainerEl);
     if (document.activeElement && DomUtils.isEditable(document.activeElement)) {
       return new InsertMode({
         singleton: "post-find-mode/focus-input",

lib/dom_utils.js

@@ -79,22 +79,6 @@ const DomUtils = {
     }
   },
 
-  //
-  // Adds a list of elements to a new container div, and adds that to the page.
-  // Returns the container div.
-  //
-  // Note that adding these nodes all at once (via a parent div) is significantly faster than
-  // one-by-one.
-  addElementsToPage(elements, containerOptions) {
-    // Don't create a new container if we already have one.
-    const parent = document.getElementById(containerOptions.id) || this.createElement("div");
-    if (containerOptions.id != null) parent.id = containerOptions.id;
-    if (containerOptions.className != null) parent.className = containerOptions.className;
-    for (const el of elements) parent.appendChild(el);
-    document.documentElement.appendChild(parent);
-    return parent;
-  },
-
   //
   // Remove an element from its DOM tree.
   //

tests/dom_tests/dom_tests.js

@@ -72,9 +72,9 @@ const createGeneralHintTests = (isFilteredMode) => {
 
     should("create hints when activated, discard them when deactivated", () => {
       const mode = activateLinkHintsMode();
-      assert.isFalse(mode.hintMarkerContainingDiv == null);
+      assert.isFalse(mode.containerEl == null);
       mode.deactivateMode();
-      assert.isTrue(mode.hintMarkerContainingDiv == null);
+      assert.isTrue(mode.containerEl == null);
     });
 
     should("position items correctly", () => {