Closed Bug 1660973 Opened 4 years ago Closed 4 years ago

Overflow badge hides part of the DOM

Categories

(DevTools :: Inspector, defect, P1)

defect

Tracking

(firefox83 fixed)

RESOLVED FIXED
83 Branch
Tracking Status
firefox83 --- fixed

People

(Reporter: mbalfanz, Assigned: gl)

References

(Blocks 2 open bugs)

Details

Attachments

(2 files)

Attached video Kapture 2020-08-24 at 18.20.48.mp4 (deleted) —

STR:

  1. visit a page like https://bugzilla.mozilla.org/show_bug.cgi?id=1660773
  2. open devtools using a keyboard shortcut without selecting a specific element
  3. expand the markup tree until you see main#bugzilla-body
  4. click the scroll badge on that element

ER: the subtree should be expanded with the overflow causing nodes highlighted
AR: when clicking the badge before expanding, the subtree is empty

See video attached

Severity: -- → S3
Priority: -- → P1

So, it turns out that the bug is happening due to the function showNode. A much simpler example of the bug can be reproduced here: https://codepen.io/20manas/full/KKzWYpG

If the scroll badge is clicked while ensuring that the markup containers of the children of the scrollable badge have not been created, it can be seen that the element is not revealed properly.

When we refresh the page, add .fixed to #child1, and repeat the above step, we see that the overflow causing element is revealed properly.

The difference is that in the first case, the overflow causing element was not the immediate child of the scrollable element, whereas in the second case, it was.

From what I can tell, it seems like showNode is working properly when the markup containers of the parents of the node to be revealed have already been created and when they're not created, it doesn't function properly. Except for our use of it, showNode is always used as a response to emits and in each of those emits, there are a lot of other things that are also happening. All these things are not happening when onScrollableBadgeClick is called, which might be causing the issue.

Assignee: nobody → gl
Status: NEW → ASSIGNED

The problem with getOverflowCausingElements seems to be that it returned an array
of nodes that weren't properly attached to their parents.

To mitigate this, we call attachElements on the entire list of nodes and
return a disconnectedNodeArray which ensures that the returned nodes
are properly attached and therefore has all their parent node information
when calling showNode.

Blocks: 1669257
Pushed by gluong@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/7129fadb6d1e [devtools] Return a disconnectedNodeArray for getOverflowCausingElements. r=jdescottes,devtools-backward-compat-reviewers
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → 83 Branch
QA Whiteboard: [qa-83b-p2]

Trying to reproduce and verify this fix but using the steps from comment 0 I don't see the scroll badge in inspector. Are there some prefs I need to enable to get it to show? Also is this only available in Nightly?

Flags: needinfo?(gl)

(In reply to Bogdan Maris [:bogdan_maris], Release Desktop QA from comment #6)

Trying to reproduce and verify this fix but using the steps from comment 0 I don't see the scroll badge in inspector. Are there some prefs I need to enable to get it to show? Also is this only available in Nightly?

Can you try it with https://codepen.io/20manas/full/KKzWYpG?

Flags: needinfo?(gl)

(In reply to Gabriel [:gl] (ΦωΦ) from comment #7)

(In reply to Bogdan Maris [:bogdan_maris], Release Desktop QA from comment #6)

Trying to reproduce and verify this fix but using the steps from comment 0 I don't see the scroll badge in inspector. Are there some prefs I need to enable to get it to show? Also is this only available in Nightly?

Can you try it with https://codepen.io/20manas/full/KKzWYpG?

I tried but it still does not appear in old Nightly (2020-08-25) and also in latest Nightly from today. I also flipped devtools.overflow.debugging.enabled to enable this feature but I can only see event and flex badges. Are there any more prefs to change in order to make it work?

Flags: needinfo?(gl)

You shouldn't have to flip any prefs since the pref is already enabled by default. You should be able to see the "scroll" badge for the id="top" element in the codepen example.

Flags: needinfo?(gl)
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: