Open Bug 1583654 Opened 4 months ago Updated 3 months ago

Grid view in Inspect Element is not persistent across refreshes

Categories

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

defect

Tracking

(firefox-esr60 unaffected, firefox-esr68 unaffected, firefox67 unaffected, firefox68 unaffected, firefox69 wontfix, firefox70 wontfix, firefox71 fixed)

ASSIGNED
Tracking Status
firefox-esr60 --- unaffected
firefox-esr68 --- unaffected
firefox67 --- unaffected
firefox68 --- unaffected
firefox69 --- wontfix
firefox70 --- wontfix
firefox71 --- fixed

People

(Reporter: developercary, Assigned: gl)

References

(Regression)

Details

(Keywords: leave-open, regression)

Attachments

(1 file)

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.

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

Component: Untriaged → Inspector
Product: Firefox → DevTools

Thanks for filing this Cary.

I tried the following but this worked fine:

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.

Type: enhancement → defect
Flags: needinfo?(developercary)

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.

Flags: needinfo?(developercary)

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.

Status: UNCONFIRMED → NEW
Component: Inspector → Inspector: Layout
Ever confirmed: true
Keywords: regression

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.

Flags: needinfo?(gl)
Regressed by: 1550519
Assignee: nobody → gl
Status: NEW → ASSIGNED
Flags: needinfo?(gl)
Priority: -- → P3

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.

Keywords: leave-open
Pushed by gluong@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/abfee71f01b0
Use the subgridToParentMap to check that the node was previously a subgrid. r=pbro

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

Flags: needinfo?(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.

Flags: needinfo?(gl) → needinfo?(pascalc)

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

Flags: needinfo?(pascalc) → needinfo?(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.

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