Skip to content

Commit 13b2bc9

Browse files
JulianDealphilc
authored andcommitted
Vimium is now using top z-index layer
1 parent 0fec088 commit 13b2bc9

File tree

3 files changed

+29
-38
lines changed

3 files changed

+29
-38
lines changed

content_scripts/link_hints.js

Lines changed: 17 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -379,7 +379,7 @@ class LinkHintsMode {
379379
this.stableSortCount = 0;
380380
this.hintMarkers = hintDescriptors.map((desc) => this.createMarkerFor(desc));
381381
this.markerMatcher = Settings.get("filterLinkHints") ? new FilterHints() : new AlphabetHints();
382-
this.markerMatcher.fillInMarkers(this.hintMarkers, this.getNextZIndex.bind(this));
382+
this.markerMatcher.fillInMarkers(this.hintMarkers);
383383

384384
this.hintMode = new Mode();
385385
this.hintMode.init({
@@ -402,11 +402,16 @@ class LinkHintsMode {
402402
}
403403
});
404404

405+
this.renderHints();
406+
this.setIndicator();
407+
}
408+
409+
renderHints() {
405410
// Append these markers as top level children instead of as child nodes to the link itself,
406411
// because some clickable elements cannot contain children, e.g. submit buttons.
407412
this.hintMarkerContainingDiv = DomUtils.addElementsToPage(
408413
this.hintMarkers.filter((m) => m.isLocalMarker()).map((m) => m.element),
409-
{ id: "vimiumHintMarkerContainer", className: "vimiumReset" },
414+
{ id: "vimiumHintMarkerContainer", className: "vimiumReset" }
410415
);
411416

412417
// TODO(philc): 2024-03-27 Remove this hasPopoverSupport check once Firefox has popover support.
@@ -431,16 +436,6 @@ class LinkHintsMode {
431436
this.setIndicator();
432437
}
433438

434-
// Increments and returns the Z index that should be used for the next hint marker on the page.
435-
getNextZIndex() {
436-
if (this.currentZIndex == null) {
437-
// This is the starting z-index value; it produces z-index values which are greater than all
438-
// of the other z-index values used by Vimium.
439-
this.currentZIndex = 2140000000;
440-
}
441-
return ++this.currentZIndex;
442-
}
443-
444439
setOpenLinkMode(mode, shouldPropagateToOtherFrames) {
445440
this.mode = mode;
446441
if (shouldPropagateToOtherFrames == null) {
@@ -476,7 +471,6 @@ class LinkHintsMode {
476471
el.style.left = localHint.rect.left + "px";
477472
el.style.top = localHint.rect.top + "px";
478473
// Each hint marker is assigned a different z-index.
479-
el.style.zIndex = this.getNextZIndex();
480474
el.className = "vimiumReset internalVimiumHintMarker vimiumHintMarker";
481475
Object.assign(marker, {
482476
element: el,
@@ -589,7 +583,6 @@ class LinkHintsMode {
589583
const { linksMatched, userMightOverType } = this.markerMatcher.getMatchingHints(
590584
this.hintMarkers,
591585
tabCount,
592-
this.getNextZIndex.bind(this),
593586
);
594587
if (linksMatched.length === 0) {
595588
this.deactivateMode();
@@ -622,7 +615,6 @@ class LinkHintsMode {
622615
const localHintMarkers = this.hintMarkers.filter((m) =>
623616
m.isLocalMarker() && (m.element.style.display !== "none")
624617
);
625-
626618
// Fill in the markers' rects, if necessary.
627619
for (const marker of localHintMarkers) {
628620
if (marker.markerRect == null) {
@@ -659,17 +651,16 @@ class LinkHintsMode {
659651
}
660652
}
661653

662-
// Rotate the z-indexes within each stack.
663-
for (const stack of stacks) {
654+
const newMarkers = []
655+
for (let stack of stacks) {
664656
if (stack.length > 1) {
665-
const zIndexes = stack.map((marker) => marker.element.style.zIndex);
666-
zIndexes.push(zIndexes[0]);
667-
for (let index = 0; index < stack.length; index++) {
668-
const marker = stack[index];
669-
marker.element.style.zIndex = zIndexes[index + 1];
670-
}
657+
// Push the last element to the beginning.
658+
stack = stack.splice(-1, 1).concat(stack)
671659
}
660+
newMarkers.push(...stack)
672661
}
662+
this.hintMarkers = newMarkers;
663+
this.renderHints();
673664
}
674665

675666
// When only one hint remains, activate it in the appropriate way. The current frame may or may
@@ -877,7 +868,7 @@ class FilterHints {
877868
marker.element.innerHTML = spanWrap(caption);
878869
}
879870

880-
fillInMarkers(hintMarkers, getNextZIndex) {
871+
fillInMarkers(hintMarkers) {
881872
for (const marker of hintMarkers) {
882873
if (marker.isLocalMarker()) {
883874
this.renderMarker(marker);
@@ -886,10 +877,10 @@ class FilterHints {
886877

887878
// We use getMatchingHints() here (although we know that all of the hints will match) to get an
888879
// order on the hints and highlight the first one.
889-
return this.getMatchingHints(hintMarkers, 0, getNextZIndex);
880+
return this.getMatchingHints(hintMarkers, 0);
890881
}
891882

892-
getMatchingHints(hintMarkers, tabCount, getNextZIndex) {
883+
getMatchingHints(hintMarkers, tabCount) {
893884
// At this point, linkTextKeystrokeQueue and hintKeystrokeQueue have been updated to reflect the
894885
// latest input. Use them to filter the link hints accordingly.
895886
const matchString = this.hintKeystrokeQueue.join("");
@@ -910,7 +901,6 @@ class FilterHints {
910901

911902
if (this.activeHintMarker?.element) {
912903
this.activeHintMarker.element.classList.add("vimiumActiveHintMarker");
913-
this.activeHintMarker.element.style.zIndex = getNextZIndex();
914904
}
915905

916906
return {

content_scripts/vimium.css

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -3,10 +3,9 @@
33
* don't use the same CSS class names that the page is using, so the page's CSS doesn't mess with
44
* the style of our Vimium dialogs.
55
*
6-
* The z-indexes of Vimium elements are very large, because we always want them to show on top. We
7-
* know that Chrome supports z-index values up to about 2^31. The values we use are large numbers
8-
* approaching that bound. However, we must leave headroom for link hints. Hint marker z-indexes
9-
* start at 2140000001.
6+
* We use the maximum z-index value for all Vimium elements to guarantee that they always appear on
7+
* top. Chrome supports z-index values up to 2,147,483,647 (= 2^31 - 1). We utilize the maximum
8+
* z-index value allowable to ensure Vimium elements have precedence over all other page elements.
109
*/
1110

1211
/*
@@ -60,7 +59,7 @@ tr.vimiumReset {
6059
vertical-align: baseline;
6160
white-space: normal;
6261
width: auto;
63-
z-index: 2140000000; /* Vimium's reference z-index value. */
62+
z-index: 2147483647;
6463
}
6564

6665
thead.vimiumReset, tbody.vimiumReset {
@@ -90,6 +89,7 @@ div.internalVimiumHintMarker {
9089
border: solid 1px #C38A22;
9190
border-radius: 3px;
9291
box-shadow: 0px 3px 7px 0px rgba(0, 0, 0, 0.3);
92+
z-index: 2147483647;
9393
}
9494

9595
div.internalVimiumHintMarker span {
@@ -153,7 +153,7 @@ iframe.vimiumHelpDialogFrame {
153153
display: block;
154154
position: fixed;
155155
border: none;
156-
z-index: 2139999997; /* Three less than the reference value. */
156+
z-index: 2147483647;
157157
}
158158

159159
div#vimiumHelpDialogContainer {
@@ -320,7 +320,7 @@ div.vimiumHUD {
320320
border-radius: 4px;
321321
box-shadow: 0px 2px 10px rgba(0, 0, 0, 0.8);
322322
border: 1px solid #aaa;
323-
z-index: 2139999999;
323+
z-index: 2147483647;
324324
}
325325

326326
iframe.vimiumHUDFrame {
@@ -336,7 +336,7 @@ iframe.vimiumHUDFrame {
336336
right: 20px;
337337
margin: 0 0 0 -40%;
338338
border: none;
339-
z-index: 2139999998; /* Two less than the reference value. */
339+
z-index: 2147483647;
340340
opacity: 0;
341341
}
342342

@@ -413,15 +413,15 @@ iframe.vomnibarFrame {
413413
margin: 0 0 0 -40%;
414414
border: none;
415415
font-family: sans-serif;
416-
z-index: 2139999998; /* Two less than the reference value. */
416+
z-index: 2147483647;
417417
}
418418

419419
div.vimiumFlash {
420420
box-shadow: 0px 0px 4px 2px #4183C4;
421421
padding: 1px;
422422
background-color: transparent;
423423
position: absolute;
424-
z-index: 2140000000;
424+
z-index: 2147483647;
425425
}
426426

427427
/* UIComponent CSS */

lib/dom_utils.js

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -86,7 +86,8 @@ const DomUtils = {
8686
// Note that adding these nodes all at once (via a parent div) is significantly faster than
8787
// one-by-one.
8888
addElementsToPage(elements, containerOptions) {
89-
const parent = this.createElement("div");
89+
// Don't create a new container if we already have one.
90+
const parent = document.getElementById(containerOptions.id) || this.createElement("div");
9091
if (containerOptions.id != null) parent.id = containerOptions.id;
9192
if (containerOptions.className != null) parent.className = containerOptions.className;
9293
for (const el of elements) parent.appendChild(el);

0 commit comments

Comments
 (0)