1583654 - Grid view in Inspect Element is not persistent across refreshes

Status: RESOLVED FIXED

(DevTools :: Inspector: Layout, defect, P3)

Description

[Cary Hartline]

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_14_6) AppleWebKit/605.1.15 (KHTML, like Gecko) Version/13.0 Safari/605.1.15

Steps to reproduce:

1. Open Inspect Element
2. Click on "grid" tag next to an element to turn on Grid visualization
3. Refreshed page.

Actual results:

The grid view toggled off and I need to click on it again to see the grid view again. This is different from the similar flex view which persists across refreshes.

Expected results:

The grid view should have similar results to the flex view by way of persisting on throughout refreshes. This is important for web developers that would like to see how the grid changes based on changes in their CSS.

Comment 1

[BugBot [:suhaib / :marco/ :calixte]]

Bugbug thinks this bug should belong to this component, but please revert this change in case of error.

Comment 2

[Patrick Brosset <:pbro>]

Thanks for filing this Cary.

I tried the following but this worked fine:

• Open https://developer.mozilla.org/en-US/
• Open the inspector
• Find the element with id newsletterForm
• Click on the "grid" badge next to it
• Reload the page

The grid lines re-appeared automatically after reload.
So there must be something different about what you were doing. Would you be able to provide a test case where this reproduces consistently?

One idea I have is: if the grid element changes every time you refresh the page, maybe that would explain it. Maybe it gets a different ID every time.

Comment 3

[Cary Hartline]

You are correct that this does work as expected on every website I can find. As well, it works as expected for the website on my own machine. The only place that I can find this issue is on the live version of the website http://caryhartline.github.io. The fact that it works on my own computer and not the live version is surprising. I have tested this in both Firefox Developer Edition and Firefox.

This website is simply static HTML with a linked stylesheet so it should not have the issue of the element changing between refreshes.

Another detail about the issue is that, if I select both grid overlays on that website, a refresh will disable the upper-level grid and a second refresh will disable the lower-level grid. If I select one grid or the other, that one grid will be disabled on refresh.

Comment 4

[Patrick Brosset <:pbro>]

Thanks Cary. I was able to reproduce the bug thanks to your page. It does not always happen for me, but sufficiently often that investigation should be possible.
Here are the steps I took:

1. Open Firefox,
2. open https://caryhartline.github.io/
3. open devtools
4. toggle the first grid from the grid inspector
5. reload the page

The grid highlighter sometimes does not re-appear after reload.
When it does, I found that doing the following helps trigger the bug again:

• close devtools
• refresh the page
• re-open devtools and do steps 4 and 5 again.

Now, when the bug does happen, I can usually see the grid highlighter appear quickly on reload, and then disappear right after.
There are no JS errors in the Browser Console.

Comment 5

[Patrick Brosset <:pbro>]

Regression range identified: https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=11757ba8621a71635a2c5be25c052a58077e7b43&tochange=9eec249083d2e3f853b58a9f5f115999e0718342

This is the regressing bug: bug 1550519.

Gabriel, can you check what in bug 1550519 could have caused this? What's odd is that this does not occur on all grids.

Comment 6

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

Attachment: Bug 1583654 - Use the subgridToParentMap to check that the node was previously a subgrid. r=pbro

The previous condition didn't fully check that the grid node was previously a subgrid.
So, we run into a scenario where we refresh the page and a "display-change" event is hit
after a new root is loaded, and the grid highlighter is restored and hidden because
the check will pass as long as the node is a grid container.

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/abfee71f01b0
Use the subgridToParentMap to check that the node was previously a subgrid. r=pbro

Comment 8

[Arthur Iakab [arthur_iakab]]

https://hg.mozilla.org/mozilla-central/rev/abfee71f01b0

Comment 9

[Pascal Chevrel:pascalc (PTO until April 26)]

Gabriel, this had been on Nightly for 2 weeks and we ship subgrids in 71, it seems to be a good candidate for a beta uplift. Could you prepare the uplift request if you agree with my assessment? Thanks

Comment 10

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

(In reply to Pascal Chevrel:pascalc from comment #9)

Gabriel, this had been on Nightly for 2 weeks and we ship subgrids in 71, it seems to be a good candidate for a beta uplift. Could you prepare the uplift request if you agree with my assessment? Thanks

Looking at https://hg.mozilla.org/mozilla-central/rev/abfee71f01b0 I believe this did land in 71. So, I am wondering if we need the uplift.

Comment 11

[Pascal Chevrel:pascalc (PTO until April 26)]

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

(In reply to Pascal Chevrel:pascalc from comment #9)

Gabriel, this had been on Nightly for 2 weeks and we ship subgrids in 71, it seems to be a good candidate for a beta uplift. Could you prepare the uplift request if you agree with my assessment? Thanks

Looking at https://hg.mozilla.org/mozilla-central/rev/abfee71f01b0 I believe this did land in 71. So, I am wondering if we need the uplift.

True, I don't why 71 was not marked as fixed when the patch landed, maybe because of the leave-open keyword? I am marking 71 as fixed in this bug. Should that bug remain open?

Comment 12

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

(In reply to Pascal Chevrel:pascalc from comment #11)

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

(In reply to Pascal Chevrel:pascalc from comment #9)

Gabriel, this had been on Nightly for 2 weeks and we ship subgrids in 71, it seems to be a good candidate for a beta uplift. Could you prepare the uplift request if you agree with my assessment? Thanks

Looking at https://hg.mozilla.org/mozilla-central/rev/abfee71f01b0 I believe this did land in 71. So, I am wondering if we need the uplift.

True, I don't why 71 was not marked as fixed when the patch landed, maybe because of the leave-open keyword? I am marking 71 as fixed in this bug. Should that bug remain open?

Yes, I left it open to add an additional unit test for it.

Comment 13

[BugBot [:suhaib / :marco/ :calixte]]

The leave-open keyword is there and there is no activity for 6 months.
:rcaliman, maybe it's time to close this bug?