Closed Bug 1608143 Opened 5 years ago Closed 4 years ago

Add a platform API for DevTools to return a list of child nodes that caused an overflow

Categories

(Core :: CSS Parsing and Computation, enhancement)

enhancement
Not set
normal

Tracking

()

RESOLVED DUPLICATE of bug 1639893

People

(Reporter: pbro, Assigned: manas.khurana20)

References

(Blocks 1 open bug)

Details

Attachments

(1 obsolete file)

In DevTools, we show when elements have an overflow area, with a little [scrollable] badge inside of the inspector (see screenshot).
This is super useful when people are trying to debug unwanted scrollbars on their pages. It tells them where to start looking for problems.

We want to expand on this feature and make it even easier for people to debug this common problem. There are 2 things we want to do:

  • show where the scrollable area is in the page
  • list the elements that caused the overflow to occur.

This bug is about the latter.

I attempted to do this using JavaScript: my idea was to go through all descendants of the scrollable container, set them to display:contents, one by one, and check every time if that changed the scrollable area size. If it did, then that descendant is responsible for the overflow. Here's a prototype piece of code for this: https://gist.github.com/captainbrosset/4e6a05637acb0d15cda29bab01173c4e

While talking to Mats about this, he raised concerns:

Changing display to display:contents is an invasive change. It could change the overall layout of the page in ways that invalidates any observations you can do on the new box tree. So while this might work for simple examples, I don't think it will work reliably in general.

He also suggested a platform way to do this:

Adding some Element API that would return a list of child elements that caused the overflow.

To be clear, we would want to have a way to detect elements that really caused the overflow to happen, not the elements that happen to be inside the overflow, but that didn't initially caused it.

See Also: → 1608140
No longer depends on: 1608140

I'm assuming you also want to know when text is what causes the overflow, right? That's not something you can do with your approach (unless you remove and re-add the text, and that becomes hacky really fast)

Summary: Add a platform API for DevTools to return a list of child elements that caused an overflow → Add a platform API for DevTools to return a list of child nodes that caused an overflow

That would indeed make it even more useful for our use case.

Blocks: 1529866

From Daniel Holbert on bug 1534334:

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.

I was wondering how exactly can it be determined that an element caused an overflow and does not just happen to be inside the overflow, unless we pinpoint the specific CSS rules which caused it. Pinpointing rules which caused the overflow, as has been noted in bug 1534334, is quite a complex issue.

Flags: needinfo?(patrickbrosset+bugzilla)

(In reply to manas from comment #5)

I was wondering how exactly can it be determined that an element caused an overflow and does not just happen to be inside the overflow, unless we pinpoint the specific CSS rules which caused it. Pinpointing rules which caused the overflow, as has been noted in bug 1534334, is quite a complex issue.

I'll answer for Patrick. The goal of this bug is to identify the child that is too large to fit in the visible area of its parent. There are lots of CSS rules that affect the size of the child element and this API will not attempt to analyze that. When devtools uses this API to visually highlight which child is causing the parent to generate a scrollbar (Bug 1529866), the devtools user will have many tools available to determine why that child is the size that it is.

Flags: needinfo?(patrickbrosset+bugzilla)
Blocks: 1638000
Blocks: 1639893
Assignee: nobody → manas.khurana20
Status: NEW → ASSIGNED

The Layout team has proposed that this method become a method on InspectorUtils, instead of on Element. The reasoning is that the necessary analysis can be done after the frame tree is constructed and doesn't need to be done during reflow itself. The implementation in LayoutUtils will be easier to structure as a frame tree traversal.

Additionally, the Layout team has made a compelling argument that the method should return all child elements that overflow the visible area.

Manas, the easiest thing to do is to mark the existing patch abandoned on Phabricator, and build a new patch based around changes to InspectorUtils.webidl, .h, and .cpp. The webidl signature will be [NewObject] NodeList getOverflowingChildrenOfElement(Element element); and won't need the [ChromeOnly, Pure] qualifier ahead of the declaration since all of InspectorUtils is chrome-only.

Flags: needinfo?(manas.khurana20)
Flags: needinfo?(manas.khurana20)
Attachment #9153634 - Attachment is obsolete: true

The stub implementation will be completed in Bug 1639893.

Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → DUPLICATE
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: