Note: There are a few cases of duplicates in user autocompletion which are being worked on.

grid inspector fails on grid containers with no layout

RESOLVED FIXED in Firefox 55

Status

()

Firefox
Developer Tools: Inspector
P3
normal
RESOLVED FIXED
3 months ago
2 months ago

People

(Reporter: jdescottes, Assigned: pbro)

Tracking

(Blocks: 2 bugs)

unspecified
Firefox 55
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox55 fixed)

Details

(URL)

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(2 attachments)

(Reporter)

Description

3 months ago
Created attachment 8860388 [details]
test_empty_grid_container.html

STRs: 
- open attached html test case
- open inspector
- select layout panel, scroll down to grids
- click checkbox for the second ".grid" element

ER: the checkbox should be checked
AR: nothing happens

The following exception is logged:

>   Message: TypeError: fragment.rows.lines[0] is undefined
>   Stack:
>     getFirstRowLinePos@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/server/actors/highlighters/css-grid.js:880:5
> renderFragment@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/server/actors/highlighters/css-grid.js:916:22
> _update@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/server/actors/highlighters/css-grid.js:669: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:502: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:1759:15
> receiveMessage@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/shared/transport/transport.js:761:7
(Reporter)

Updated

3 months ago

Updated

3 months ago
Blocks: 1347964
Priority: -- → P3
(Assignee)

Comment 1

3 months ago
I'll take this one. Simple fix, we just need to check that grid fragments actually contain tracks before drawing them.
Assignee: nobody → pbrosset
Status: NEW → ASSIGNED
(Assignee)

Comment 2

3 months ago
Bug 1297072 is almost ready to land and is changing quite a lot the file that I need to fix here.
So let's just depend on that bug.
Depends on: 1297072
Comment hidden (mozreview-request)
(Assignee)

Comment 4

3 months ago
https://treeherder.mozilla.org/#/jobs?repo=try&revision=ddf2b45c45831ab7eee211c0c57b62e4169182d2&group_state=expanded
(Reporter)

Comment 5

3 months ago
mozreview-review
Comment on attachment 8864415 [details]
Bug 1358479 - Check if a grid fragment is valid before highlighting it;

https://reviewboard.mozilla.org/r/136106/#review139122

Great thanks!

::: commit-message-c4e3b:3
(Diff revision 1)
> +Bug 1358479 - Check if a grid fragment is valid before highlighting it; r=jdescottes
> +
> +Grid fragments returned by the chrome-only el.getGtidFragments() API

mega-nit: getGtidFragments -> getGridFragments

::: devtools/server/actors/highlighters/css-grid.js:756
(Diff revision 1)
>      this.updateCurrentMatrix();
>  
>      // Start drawing the grid fragments.
>      for (let i = 0; i < this.gridData.length; i++) {
>        let fragment = this.gridData[i];
> +      if (!this.isValidFragment(fragment)) {

Maybe move this check inside of renderFragment?

Although renderFragment is only called here so it doesn't really matter at the moment.
Attachment #8864415 - Flags: review?(jdescottes) → review+
Comment hidden (mozreview-request)

Comment 7

3 months ago
Pushed by pbrosset@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/73696a2f5ec6
Check if a grid fragment is valid before highlighting it; r=jdescottes
https://hg.mozilla.org/mozilla-central/rev/73696a2f5ec6
Status: ASSIGNED → RESOLVED
Last Resolved: 3 months ago
status-firefox55: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 55

Comment 9

2 months ago
I have successfully reproduced the bug in firefox Nightly 55.0a1 (2017-04-21) (32-bit) with windows 7 (32 bit) 

Verified as fixed with latest nightly 55.0a1 (2017-05-13) (32-bit)

Build ID:   20170513030205
Mozilla/5.0 (Windows NT 10.0; rv:55.0) Gecko/20100101 Firefox/55.0

[testday-20170512]
You need to log in before you can comment on or make changes to this bug.