1479887 - Grid Inspector highlighter can cause scrollbars to toggle on and off rapidly

Status: RESOLVED FIXED

(DevTools :: Inspector, defect, P2)

Description

[Brad Werth [:bradwerth]]

Attachment: grid shudder.gif

Steps to Reproduce:

1) Navigate to https://gridbyexample.com/examples/code/example29.html
2) Open Devtools Inspector.
3) Resize the window so that there is no vertical scrollbar visible.
4) In the Inspector, select the <div class="wrapper"> element, and in the Layout panel, turn on the Overlay Grid.
5) In the Overlay Grid, move the cursor over one of the cells in the right-most column. The overlay will be drawn and the scrollbars will rapidly toggle between being shown and being hidden.

Comment 1

[Patrick Brosset <:pbro>]

It's funny because we want to fix this for the Overflow Debugging feature, which is a feature that will help people debug unwanted scrollbars. But at the same time we do need the feature to debug this particular issue :)
So, I hacked up a rough version of it in DevTools to see if I could use this to help me investigate, and it worked.

The node that seems to be causing the problem is the <div id="css-grid-cell-infobar-container">.

It moves around the screen to always be displayed near the thing it points to. Here it points to a grid cell.
When that grid cell is near the edge of the viewport, the infobar sometimes overflows the viewport, causing the scrollbar.

If you add background:red to the :-moz-native-anonymous [class$=infobar-container] {} CSS rule in /devtools/server/actors/highlighters.css then you can see the problem. It's not visible otherwise because this element's content is shifted to the left with a 50% relative positioning technique.

One solution would be to make the container of the infobar be overflow:hidden, but that will break other things: the entire highlighter won't always be visible when scrolling the page.

So instead we need a way to make sure the infobar is always positioned entirely inside the viewport.
Fixing this here will fix other infobars too, in particular the one displayed in the box-model highlighter, which also has similar problems (see bug 1444028).

The infobar code is in moveInfobar in /devtools/server/actors/highlighters/utils/markup.js.

Comment 2

[Patrick Brosset <:pbro>]

After looking at this further it looks like the logic for positioning the infobar is as follows:

• it is sized such that it would in theory fit in the viewport
• its left-most boundary is aligned with the center of the element it points ti
• it is then moved to the left so its center point is aligned with the center of the element. This is done by setting this on the infobar container's first child: positioned:relative;left:-50%.

The problem with this is that this leaves the infobar container where it was, so not moved to the left, so sometimes overflowing the page.

If instead we used a css transform, this might fix the issue.
Here's something that seems to work:

--- a/devtools/server/actors/highlighters.css
+++ b/devtools/server/actors/highlighters.css
@@ -137,19 +137,16 @@
 
   font: var(--highlighter-font-family);
   font-size: var(--highlighter-font-size);
 }
 
 :-moz-native-anonymous [class$=infobar] {
   position: relative;
 
-  /* Centering the infobar in the container */
-  left: -50%;
-
   padding: 5px;
   min-width: 75px;
 
   border-radius: 3px;
   background: var(--highlighter-bubble-background-color) no-repeat padding-box;
 
   color: var(--highlighter-bubble-text-color);
   text-shadow: none;

--- a/devtools/server/actors/highlighters/utils/markup.js
+++ b/devtools/server/actors/highlighters/utils/markup.js
@@ -672,13 +672,13 @@ function moveInfobar(container, bounds, 
   // We need to scale the infobar Independently from the highlighter's container;
   // otherwise the `position: fixed` won't work, since "any value other than `none` for
   // the transform, results in the creation of both a stacking context and a containing
   // block. The object acts as a containing block for fixed positioned descendants."
   // (See https://www.w3.org/TR/css-transforms-1/#transform-rendering)
   container.setAttribute("style", `
     position:${position};
     transform-origin: 0 0;
-    transform: scale(${1 / zoom}) translate(${left}px, ${top}px)`);
+    transform: scale(${1 / zoom}) translate(calc(${left}px - 50%), ${top}px)`);
 
   container.setAttribute("position", positionAttribute);
 }
 exports.moveInfobar = moveInfobar;

Comment 4

[Patrick Brosset <:pbro>]

Attachment: Bug 1479887 - Use CSS transform instead of relative positioning when centering the infobar; r?rcaliman!

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/567e3f10af99
Use CSS transform instead of relative positioning when centering the infobar; r=rcaliman

Comment 6

[Raul Gurzau (:RaulG)]

https://hg.mozilla.org/mozilla-central/rev/567e3f10af99