1660973 - Overflow badge hides part of the DOM

Status: RESOLVED FIXED

(DevTools :: Inspector, defect, P1)

Description

[Martin Balfanz [:mbalfanz]]

Attachment: Kapture 2020-08-24 at 18.20.48.mp4

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

Comment 1

[manas]

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.

Comment 2

[Gabriel Luong [:gl] (ΦωΦ)]

Attachment: Bug 1660973 - [devtools] Return a disconnectedNodeArray for getOverflowCausingElements. r=jdescottes

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.

Comment 3

[Gabriel Luong [:gl] (ΦωΦ)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=a76cf0eee327b1b357a044e3ad1b82cec1399327

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/7129fadb6d1e
[devtools] Return a disconnectedNodeArray for getOverflowCausingElements. r=jdescottes,devtools-backward-compat-reviewers

Comment 5

[Narcis Beleuzu [:NarcisB]]

https://hg.mozilla.org/mozilla-central/rev/7129fadb6d1e

Comment 6

[Bogdan Maris, Desktop QA [Holiday+PTO 1-6 May]]

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?

Comment 7

[Gabriel Luong [: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?

Comment 8

[Bogdan Maris, Desktop QA [Holiday+PTO 1-6 May]]

(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?

Comment 9

[Gabriel Luong [: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.