1534334 - Make DevTools able to detect which elements/rules cause a root element to scroll

Status: RESOLVED DUPLICATE of bug 1608143

(DevTools :: Inspector, enhancement, P2)

Description

[Patrick Brosset <:pbro>]

This bug is to track the technical work required for the Overflow Debugging feature to be able to tell users which of the element(s) in the page, and which of their CSS rules cause a certain element to have a scrollbar.

Finding the element that has the scrollbar is useful, and tracked in other bugs. Here the goal is, once that element was found, to have code in DevTools (and/or platform) that is able to find the reason for it to scroll.

Comment 1

[Patrick Brosset <:pbro>]

One early attempt at doing this purely in content JS: https://gist.github.com/captainbrosset/4e6a05637acb0d15cda29bab01173c4e
To test this, you can follow these steps:

• open nightly
• open a sample page, like https://bugzilla.mozilla.org/show_bug.cgi?id=dt-flexbox
• make the window size narrow enough that a horizontal scrollbar starts to appear in the page
• open scratchpad (shift+F4)
• paste the code from that gist into scratchpad and execute it
• open the console tab in devtools, on the page, and check the result

It should give a map of CSS rules to DOM nodes.

The way this works is the following:

• find all elements below the root scrollable element
• check that they are (at least partially) in the scrollable overflow area of that root element
• if they are, get all matching CSS rules
• once all CSS rules have been found, iterate and add display:contents to each of them, one by one, and check if the root scrollable element has had its overflow area impacted by that
• if so, report this rule and corresponding element, and continue with the rest

This is not very optimal, and can run for a long time if there are many elements involved. But it does lead to conclusive results in this test case. Because it even tells you which CSS rule was responsible for the overflow, it makes it very useful for the feature we're after.

It also works nicely in this other test case: https://overflow-debugging-test2.glitch.me/

Brad, Daniel: this is where I need your platform expertise. Would this code have any chance of actually working in most use cases you think? Are there more efficient ways of doing this? Would platform layout code be better equipped to do something like this?

Comment 2

[Daniel Holbert [:dholbert]]

(In reply to Patrick Brosset <:pbro> from comment #1)

Would this code have any chance of actually working in most use cases you think?

I can think of a few issues:
(1) RE the geometry here: You're using getBoxQuads, but that doesn't reliably report the area of the box that could create scrollable overflow. It lets you pick content/padding/border/margin-box, but unfortunately, we don't consistently use the same one of those boxes for scrollable-overflow determination. E.g. this testcase:
https://bugzilla.mozilla.org/attachment.cgi?id=9043507
...has a 100px margin below each of the green boxes (inside each scrollable area), but we only consider that margin to be "scrollable" in the first case there. Chrome disagrees and considers it scrollable in all three cases. The spec says that margins are optionally includable in scrollable overflow: https://drafts.csswg.org/css-overflow-3/#scrollable So for now, there's no right answer and no great way for you to know which box to use. :(

So: bottom-line, it's ~hard to predict which getBoxQuads box you should be using in this JS code.

(2) RE the rule-reporting: I'm not sure it makes sense to use display:contents to find a "guilty" report-worthy rule... Suppose we have e.g. these rules applying to some overflowing <div class="a">:

 .a { width: 20000px;
      /* more stuff */ }
 .a { padding-left: 99999px;
      /* more stuff */ }
 .a { display: inline-block }

I would think we'd want to blame the first and/or second rules here, because they each make the div super-wide.

But from my understanding, your approach would report the third rule as "guilty", because that's the only one where setting display will have any effect. (If you add display:contents to any earlier rule, it has zero effect, because it's overridden by the display declaration in the third rule, because these rules all have the same specificity and so the last one to set display will win.)

Are there more efficient ways of doing this? Would platform layout code be better equipped to do something like this?

For the "tell me which elements have boxes that are in the overflow area" part: I think platform layout code would be best equipped to do something like this.

For the "tell me which rules cause those elements to overflow": that is a hard problem, which I don't think has an easy answer even with perfect information. Concerns:

• E.g. suppose you have a div with N different CSS rules, each of which contribute to its size -- e.g. different rules setting padding/border/height/width, and another rule that sets transform: scale(1.2) to make it slightly bigger. And maybe another rule that sets aspect-ratio once we eventually support that property. Now, suppose that this box just barely overflows, and removing any one of these rules is sufficient to avoid overflow. How do we decide which rule(s) to blame?

• In some (many?) cases, it's really the content that's to blame, not any CSS rule. E.g. suppose I have a <img> with a 1000px tall PNG, with img { border: 2px solid black }. If this barely overflows its scrollport (say, by 2px) and we have to decide which rule to blame, would it make sense to blame the border? If the author removes that and then later swaps in a different PNG that is 1024px tall, then it would overflow again, so the border wasn't really the thing "most at fault" -- it was the content-size, really.

It might be possible to come up with a "blame" system but we'd need to work through the edge cases pretty thoroughly and come up with what makes sense to report in "shared blame" scenarios like this, I think.

Comment 3

[Daniel Holbert [:dholbert]]

Two other scenarios where "blame" would be difficult to assign:

(1) An element with three different rules which set "position: absolute", "top: 60px", and "height 50px", on a child of a 100px-height scrollable containing block. This child will cause ~10px of scrollable overflow, but if you remove any one of these rules, then it won't overflow anymore. (which rule is guilty/report-worthy? all of them?)

(1a) Suppose the child's 50px height comes from its content (say, 2 lines of text, each ~25px tall, due to a large font-size which is set in another rule). Now do we blame the font-size? Or the content itself? Or all of the rules and the content as well?

(2) On platforms without overlay-scrollbars (e.g. linux & windows, and some Mac configurations), you can have a scenarios where horizontal overflow indirectly triggers a vertical scrollbar, via the following route:

• wide content causes a horizontal scrollbar to show up at the bottom
• this scrollbar at the bottom reduces the effective height of the scrollport, because it occupies some layout space.
• if the child content was nearly as tall as the scrollport-height, then the content might now vertically overflow, due to the reduced height of the scrollport.
• this causes a a vertical scrollbar to show up, despite the fact that the child height is technically less than the parent height. (becuase it's greater than the parent height, with the scrollbar included)
  Here's an example of that, with a 100px by 100px scrollport, whose children have a (collective) bounding box of 105px by 98px:
  https://jsfiddle.net/dholbert/yp90f5j2/
  "Blame" is complicated to meaningfully assign in this scenario. Interestingly, the orange element doesn't directly cause vertical overflow -- but if you remove that element or reduce its width, then the vertical scrollbar goes away! (in addition to the horizontal scrollbar).

Comment 4

[Patrick Brosset <:pbro>]

Thanks Daniel.
Sounds like finding the right element and rule to blame will be tough. Maybe we could focus on the most common cases first as a way to make this easy to implement and still useful in, say, 90% of the times.

I'm not too concerned with finding the right CSS rule to blame. I think if we manage to tell users "here are 2 elements that cause an overflow", then that's enough for DevTools. In fact, that's what we originally set out to do with this feature.
In my little experiment it seemed possible to go further and find the rule(s) to blame too, which is why I asked.

Now, I would however want us to be able to be listing the elements to blame with a high degree of certainty.
For instance, if an element is taller than its parent and overflows, I would only want DevTools to report if this element is to blame, i.e. not one of its descendant making it taller. Otherwise we'll always be reporting subtrees of elements and it will be up to the user to select them one by one until they find the culprit. And that wouldn't be much different from what they do today.
The display:contents trick seemed to be an interesting way to do this, by taking each element one by one out of the equation, but preserving its descendants.

Comment 5

[Daniel Holbert [:dholbert]]

Yeah, element-blaming seems like a reasonably tractable thing here.

I can see that display:contents helps to assign blame in some cases, but there are plenty of cases where adding display:contents would fundamentally change the layout in a way that would invalidate any inferences we might want to draw.

I think you're just using that to distinguish between "#elem is wide [regardless of its contents] because of its own styling" vs. "#elem is wide because it shrinkwraps its descendants and they are (collectively) wide". In the former case, we'd want to blame #elem; in the latter case, we'd want to blame its descendants perhaps. That's a somewhat subtle judgement to make, but I think it's one we could probably come up with some heuristics to report in platform layout code somewhere.

Comment 6

[Patrick Brosset <:pbro>]

(In reply to Daniel Holbert [:dholbert] from comment #5)

I think you're just using that to distinguish between "#elem is wide [regardless of its contents] because of its own styling" vs. "#elem is wide because it shrinkwraps its descendants and they are (collectively) wide". In the former case, we'd want to blame #elem; in the latter case, we'd want to blame its descendants perhaps.

Yes that's exactly right.

That's a somewhat subtle judgement to make, but I think it's one we could probably come up with some heuristics to report in platform layout code somewhere.

That is great to hear. Using a heuristic for this sounds good to me. As said before, we want this tool to point users in the right general direction, not necessarily tell them "this exact element is the problem, and you should change this property in this rule to fix it". I think it's enough if the tool gives them the "most interesting" element to look at.

Intrigued by how this would work on the platform, but we're not there yet. Still planning Q2 2019 at this stage which, this feature might be part of.
Thank you Daniel for the help so far.

Comment 7

[Daniel Holbert [:dholbert]]

(In reply to Patrick Brosset <:pbro> from comment #6)

Intrigued by how this would work on the platform

Conceptually, it would be some sort of traversal over the children of the scrollable element, like so:

// Reports the given frame as being "blame-worthy" for triggering a scrollbar.
void ReportBlame(frame);

void ConsiderSubtreeForOverflow(visibleArea, subtreeRoot) {
  for (curFrame in subtreeRoot.children) {
    if (curFrame scrollable overflow exceeds visibleArea) {
      if (curFrame is a leaf node ||
          curFrame has specified size/position) {
        ReportBlame(curFrame);
      } else {
        // Some descendant is probably to blame.
        ConsiderSubtreeForOverflow(curFrame);
      }
    }
  }
}

"specified size" is hand-wavy here because e.g. this element has height:auto but is still extremely tall, entirely due to its own styling:
<div style="display:grid;grid-template-rows: 20000px; border: 1px solid black">

So there will be some nuance/hackiness there, but I'm willing to believe we can produce useful results in many cases.

Comment 8

[Patrick Brosset <:pbro>]

Thanks Daniel. Very helpful as usual.
So, the next step would be to prototype this and test it against real world use cases.

Comment 9

[Patrick Brosset <:pbro>]

I recently filed bug 1608143 for doing this on the platform side instead. So let's close this. I'll copy over comment 7 though, as that seems important enough for the other bug.