Merge remote-tracking branch 'origin/master' into feat/move-tab-to-existing-window

Author: IdoKendo

.gitignore

@@ -1,11 +1 @@
-*.un~
-*.swo
-*.swp
-*.crx
-*.sublime*
-options/*.js
-node_modules/*
-dist
-jscoverage.json
-tags
-.cake_task_cache
+dist

background_scripts/main.js

@@ -584,6 +584,11 @@ const sendRequestHandlers = {
     });
   },
 
+  launchSearchQuery({ query, openInNewTab }) {
+    const disposition = openInNewTab ? "NEW_TAB" : "CURRENT_TAB";
+    chrome.search.query({ disposition, text: query });
+  },
+
   domReady(_, sender) {
     const isTopFrame = sender.frameId == 0;
     if (!isTopFrame) return;

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).
@@ -379,7 +379,7 @@ class LinkHintsMode {
     this.stableSortCount = 0;
     this.hintMarkers = hintDescriptors.map((desc) => this.createMarkerFor(desc));
     this.markerMatcher = Settings.get("filterLinkHints") ? new FilterHints() : new AlphabetHints();
-    this.markerMatcher.fillInMarkers(this.hintMarkers, this.getNextZIndex.bind(this));
+    this.markerMatcher.fillInMarkers(this.hintMarkers);
 
     this.hintMode = new Mode();
     this.hintMode.init({
@@ -402,20 +402,33 @@ class LinkHintsMode {
       }
     });
 
+    this.renderHints();
+    this.setIndicator();
+  }
+
+  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",
@@ -431,16 +444,6 @@ class LinkHintsMode {
     this.setIndicator();
   }
 
-  // Increments and returns the Z index that should be used for the next hint marker on the page.
-  getNextZIndex() {
-    if (this.currentZIndex == null) {
-      // This is the starting z-index value; it produces z-index values which are greater than all
-      // of the other z-index values used by Vimium.
-      this.currentZIndex = 2140000000;
-    }
-    return ++this.currentZIndex;
-  }
-
   setOpenLinkMode(mode, shouldPropagateToOtherFrames) {
     this.mode = mode;
     if (shouldPropagateToOtherFrames == null) {
@@ -476,7 +479,6 @@ class LinkHintsMode {
       el.style.left = localHint.rect.left + "px";
       el.style.top = localHint.rect.top + "px";
       // Each hint marker is assigned a different z-index.
-      el.style.zIndex = this.getNextZIndex();
       el.className = "vimiumReset internalVimiumHintMarker vimiumHintMarker";
       Object.assign(marker, {
         element: el,
@@ -589,7 +591,6 @@ class LinkHintsMode {
     const { linksMatched, userMightOverType } = this.markerMatcher.getMatchingHints(
       this.hintMarkers,
       tabCount,
-      this.getNextZIndex.bind(this),
     );
     if (linksMatched.length === 0) {
       this.deactivateMode();
@@ -622,7 +623,6 @@ class LinkHintsMode {
     const localHintMarkers = this.hintMarkers.filter((m) =>
       m.isLocalMarker() && (m.element.style.display !== "none")
     );
-
     // Fill in the markers' rects, if necessary.
     for (const marker of localHintMarkers) {
       if (marker.markerRect == null) {
@@ -659,17 +659,16 @@ class LinkHintsMode {
       }
     }
 
-    // Rotate the z-indexes within each stack.
-    for (const stack of stacks) {
+    const newMarkers = []
+    for (let stack of stacks) {
       if (stack.length > 1) {
-        const zIndexes = stack.map((marker) => marker.element.style.zIndex);
-        zIndexes.push(zIndexes[0]);
-        for (let index = 0; index < stack.length; index++) {
-          const marker = stack[index];
-          marker.element.style.zIndex = zIndexes[index + 1];
-        }
+        // Push the last element to the beginning.
+        stack = stack.splice(-1, 1).concat(stack)
       }
+      newMarkers.push(...stack)
     }
+    this.hintMarkers = newMarkers;
+    this.renderHints();
   }
 
   // When only one hint remains, activate it in the appropriate way. The current frame may or may
@@ -771,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;
   }
 }
 
@@ -877,7 +876,7 @@ class FilterHints {
     marker.element.innerHTML = spanWrap(caption);
   }
 
-  fillInMarkers(hintMarkers, getNextZIndex) {
+  fillInMarkers(hintMarkers) {
     for (const marker of hintMarkers) {
       if (marker.isLocalMarker()) {
         this.renderMarker(marker);
@@ -886,10 +885,10 @@ class FilterHints {
 
     // We use getMatchingHints() here (although we know that all of the hints will match) to get an
     // order on the hints and highlight the first one.
-    return this.getMatchingHints(hintMarkers, 0, getNextZIndex);
+    return this.getMatchingHints(hintMarkers, 0);
   }
 
-  getMatchingHints(hintMarkers, tabCount, getNextZIndex) {
+  getMatchingHints(hintMarkers, tabCount) {
     // At this point, linkTextKeystrokeQueue and hintKeystrokeQueue have been updated to reflect the
     // latest input. Use them to filter the link hints accordingly.
     const matchString = this.hintKeystrokeQueue.join("");
@@ -910,7 +909,6 @@ class FilterHints {
 
     if (this.activeHintMarker?.element) {
       this.activeHintMarker.element.classList.add("vimiumActiveHintMarker");
-      this.activeHintMarker.element.style.zIndex = getNextZIndex();
     }
 
     return {
@@ -927,7 +925,7 @@ class FilterHints {
       (keyChar.toLowerCase() !== keyChar) &&
       (this.linkHintNumbers.toLowerCase() !== this.linkHintNumbers.toUpperCase())
     ) {
-      // The the keyChar is upper case and the link hint "numbers" contain characters (e.g.
+      // The keyChar is upper case and the link hint "numbers" contain characters (e.g.
       // [a-zA-Z]). We don't want some upper-case letters matching hints (above) and some matching
       // text (below), so we ignore such keys.
       return;

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",

content_scripts/vimium.css

@@ -3,10 +3,9 @@
  * don't use the same CSS class names that the page is using, so the page's CSS doesn't mess with
  * the style of our Vimium dialogs.
  *
- * The z-indexes of Vimium elements are very large, because we always want them to show on top. We
- * know that Chrome supports z-index values up to about 2^31. The values we use are large numbers
- * approaching that bound. However, we must leave headroom for link hints. Hint marker z-indexes
- * start at 2140000001.
+ * We use the maximum z-index value for all Vimium elements to guarantee that they always appear on
+ * top. Chrome supports z-index values up to 2,147,483,647 (= 2^31 - 1). We utilize the maximum
+ * z-index value allowable to ensure Vimium elements have precedence over all other page elements.
  */
 
 /*
@@ -60,7 +59,7 @@ tr.vimiumReset {
   vertical-align: baseline;
   white-space: normal;
   width: auto;
-  z-index: 2140000000; /* Vimium's reference z-index value. */
+  z-index: 2147483647;
 }
 
 thead.vimiumReset, tbody.vimiumReset {
@@ -90,6 +89,7 @@ div.internalVimiumHintMarker {
   border: solid 1px #C38A22;
   border-radius: 3px;
   box-shadow: 0px 3px 7px 0px rgba(0, 0, 0, 0.3);
+  z-index: 2147483647;
 }
 
 div.internalVimiumHintMarker span {
@@ -153,7 +153,7 @@ iframe.vimiumHelpDialogFrame {
   display: block;
   position: fixed;
   border: none;
-  z-index: 2139999997; /* Three less than the reference value. */
+  z-index: 2147483647;
 }
 
 div#vimiumHelpDialogContainer {
@@ -320,7 +320,7 @@ div.vimiumHUD {
   border-radius: 4px;
   box-shadow: 0px 2px 10px rgba(0, 0, 0, 0.8);
   border: 1px solid #aaa;
-  z-index: 2139999999;
+  z-index: 2147483647;
 }
 
 iframe.vimiumHUDFrame {
@@ -336,7 +336,7 @@ iframe.vimiumHUDFrame {
   right: 20px;
   margin: 0 0 0 -40%;
   border: none;
-  z-index: 2139999998; /* Two less than the reference value. */
+  z-index: 2147483647;
   opacity: 0;
 }
 
@@ -413,15 +413,15 @@ iframe.vomnibarFrame {
   margin: 0 0 0 -40%;
   border: none;
   font-family: sans-serif;
-  z-index: 2139999998; /* Two less than the reference value. */
+  z-index: 2147483647;
 }
 
 div.vimiumFlash {
   box-shadow: 0px 0px 4px 2px #4183C4;
   padding: 1px;
   background-color: transparent;
   position: absolute;
-  z-index: 2140000000;
+  z-index: 2147483647;
 }
 
 /* UIComponent CSS */

lib/dom_utils.js

@@ -79,21 +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) {
-    const parent = 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.
   //

lib/url_utils.js

@@ -85,8 +85,8 @@ const UrlUtils = {
     return false;
   },
 
-  // Converts :string into a Google search if it's not already a URL. We don't bother with escaping
-  // characters as Chrome will do that for us.
+  // Converts string into a full URL if it's not already one. We don't escape characters as the
+  // browser will do that for us.
   async convertToUrl(string) {
     string = string.trim();
 
@@ -98,7 +98,9 @@ const UrlUtils = {
     } else if (await this.isUrl(string)) {
       return this.createFullUrl(string);
     } else {
-      return this.createSearchUrl(string);
+      const message = `convertToUrl: can't convert "${string}" into a URL. This shouldn't happen.`;
+      console.error(message);
+      return null;
     }
   },
 
@@ -125,13 +127,8 @@ const UrlUtils = {
     }
   },
 
-  // Create a search URL from the given :query (using either the provided search URL, or the default
-  // one). It would be better to pull the default search engine from Chrome itself. However, Chrome
-  // does not provide an API for doing so.
+  // Create a search URL from the given :query using the provided search URL.
   createSearchUrl(query, searchUrl) {
-    if (searchUrl == null) {
-      searchUrl = Settings.get("searchUrl") || Settings.defaultOptions.defaultSearchUrl;
-    }
     if (!["%s", "%S"].some((token) => searchUrl.indexOf(token) >= 0)) {
       searchUrl += "%s";
     }
@@ -142,7 +139,9 @@ const UrlUtils = {
   // Map a search query to its URL encoded form. The query may be either a string or an array of
   // strings. E.g. "BBC Sport" -> "BBC%20Sport".
   createSearchQuery(query) {
-    if (typeof query === "string") query = query.split(/\s+/);
+    if (typeof query === "string") {
+      query = query.split(/\s+/);
+    }
     return query.map(encodeURIComponent).join("%20");
   },
 };

manifest.json

@@ -30,7 +30,8 @@
     // loads. This permission was required when moving to manifest V3.
     "scripting",
     "favicon", // The favicon permission is not yet supported by Firefox.
-    "webNavigation"
+    "webNavigation",
+    "search"
   ],
   "content_scripts": [
     {

pages/completion_engines.html

@@ -46,7 +46,7 @@
       </p>
       <header>Available Completion Engines</header>
       <p>
-        Search completion is available in this version of Vimium for the the following custom search engines.
+        Search completion is available in this version of Vimium for the following custom search engines.
       </p>
       <p>
         <dl id="engineList"></dl>