[Affected versions]: - latest Nightly 56.0a1 - Beta 55.0b5 (20170626165718) [Affected platforms]: - Windows 10 x64 - macOS 10.12.5 - Ubuntu 16.04 x64 LTS [Steps to reproduce]: 1. Start Firefox. 2. Go to: https://www.mozilla.org/en-US/developer/css-grid/?utm_source=gridtooltip&utm_medium=devtools&utm_campaign=cssgrid_layout 3. Open Inspect Element (F12) and go to the Layout tab. 4. Check "div.content.grid-content". 5. Refresh the webpage. (I've noticed that sometimes it requires more than 1 refresh to reproduce this) [Expected result]: a) The CSS Grid checkbox stays enabled in Inspector after refresh or b) The CSS Grid checkbox is disabled in the Inspector and Grids are not displayed anymore on the page. (after refresh) [Actual result]: - The CSS Grid appears enabled on the test page but "div.content.grid-content" checkbox is unchecked in Layout tab. [Regression range]: - If the right expected result is b) then it seems to be a regression, as I cannot reproduce it on 53.0a1 when this feature was first implemented. In that case I will follow up with a regression range. [Additional notes]: - Please note that with other Overlay Grids, e.g.: div.content.grid-content, div.content.grid-content this issue is not reproducible - .gif attached showing this issue

The expected result should be a) since we have fixed bug 1342310 that makes highlighted grids re-highlight themselves after a page refresh. This seems like a somewhat random init bug. When it happens, if you switch to the rule-view and then back to the layout-view, then the checkbox will become checked again.

This is most likely a race condition between the initialization of the grid list and of the state restoring done in the highlighter-overlay. The grid list should wait until the state has been restored, so that it shows the right checkbox being checked.

Patrick, are you considering this a blocker to ship the feature? Or is it rare enough that it shouldn't?

I don't think this should block shipping the feature. But I'll let Gabriel take the final decision on this.

Comment on attachment 8889605 [details] Bug 1376774 - Restore highlighter states on markup loaded prior to emitting the 'new-root' event. https://reviewboard.mozilla.org/r/160626/#review166450 Doesn't completely fix the issue for me. On http://labs.jensimmons.com/2016/examples/grid-content-1.html, after highlighting the grid and reloading the page I get the following stack trace: console.error: Message: TypeError: can't access dead object Stack: renderGridGap@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/server/actors/highlighters/css-grid.js:1527:26 renderLines@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/server/actors/highlighters/css-grid.js:1316:9 renderFragment@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/server/actors/highlighters/css-grid.js:1131:5 _update@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/server/actors/highlighters/css-grid.js:855:7 update@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/server/actors/highlighters/auto-refresh.js:240:5 _startRefreshLoop@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/server/actors/highlighters/auto-refresh.js:276:5 show@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/server/actors/highlighters/auto-refresh.js:110:5 show@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/server/actors/highlighters.js:505:12 handler@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/shared/protocol.js:1082:19 onPacket@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/server/main.js:1799:15 receiveMessage@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/shared/transport/transport.js:761:7 And the grid is not displayed after the reload. Subsequent reloads display the grid without any issue. Now that we are not waiting for for new root, maybe we are trying to redisplay the grid slightly too soon? The issue here is with the gridGapPattern, that is normally cleared on navigate, in the css grid highlighter. Calling this._clearCache() in onWillNavigate seems to fix the issue, but this requires more testing. Maybe check with zer0 to see what he thinks about that. ::: devtools/client/inspector/inspector.js:808 (Diff revision 1) > + > + this.markup.expandNode(this.selection.nodeFront); > + > + // Restore the highlighter states prior to emitting "new-root". > + yield this.highlighters.restoreGridState(); > + yield this.highlighters.restoreShapeState(); We could yield Promise.all([restoreGrid(), restoreShape()]) here.

Moving clearCache to will-navigate should be more than fine.

Comment on attachment 8890393 [details] [diff] [review] 1376774.patch Review of attachment 8890393 [details] [diff] [review]: ----------------------------------------------------------------- Looks good to me, r+ with a green try.

Pushed by gabriel.luong@gmail.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/aa4bd3b77a17 Restore the highlighter states on markup loaded prior to emitting the "new-root" event. r=jdescottes

Verified that using latest Nightly across platforms(macOS 10.12.5, Windows 7 32bit and Ubuntu 16.04 64bit), I no longer encounter this issue. I'll go ahead and mark as wontfix on Fx55 because you have to force enable the feature and the plan is to ride 56 so I doubt it's worth investing time in pushing to 55. Please feel free to correct me if I'm wrong.