Closed
Bug 1337607
Opened 9 years ago
Closed 9 years ago
display:inline-grid isn't recognized by the Grid inspector
Categories
(DevTools :: Inspector, defect, P3)
DevTools
Inspector
Tracking
(firefox54 verified)
VERIFIED
FIXED
Firefox 54
| Tracking | Status | |
|---|---|---|
| firefox54 | --- | verified |
People
(Reporter: MatsPalmgren_bugz, Assigned: MatsPalmgren_bugz)
References
(Blocks 1 open bug, )
Details
Attachments
(1 file)
|
3.10 KB,
patch
|
gl
:
review+
|
Details | Diff | Splinter Review |
STR
1. load data:text/html,<body style="display:inline-grid">Hello
2. F12
3. In the Rules view, the button to enable the Grid overlay is missing
(Changing "inline-grid" in the test to "grid" works as expected.)
EXPECTED RESULTS
All DevTools for 'grid' should also work for 'inline-grid'.
|
Assignee | |
Comment 1•9 years ago
|
||
This seems to work fine in my local build.
https://treeherder.mozilla.org/#/jobs?repo=try&revision=01c5efc7444502e9a76956ff5814a23788cdb32a
Assignee: nobody → mats
Attachment #8835769 -
Flags: review?(gl)
|
|
Assignee | |
Comment 2•9 years ago
|
||
Hmm, what's this then?
"TEST-UNEXPECTED-ERROR | /home/worker/checkouts/gecko/devtools/client/shared/output-parser.js:146:11 | Function '_parse' has a complexity of 36. (complexity)"
What's the trick to get around such nuisance?
|
||
Comment 3•9 years ago
|
||
Comment on attachment 8835769 [details] [diff] [review]
fix
Review of attachment 8835769 [details] [diff] [review]:
-----------------------------------------------------------------
We should also add an unit test for testing the toggle of the grid highlighter for 'display:inline-grid'. You should be able to replicate this and make the modifications using devtools/client/inspector/rules/test/browser_rules_grid-toggle_01.js.
Thank you for the patch, mats!
::: devtools/client/shared/output-parser.js
@@ +208,5 @@
> if (options.expectCubicBezier &&
> BEZIER_KEYWORDS.indexOf(token.text) >= 0) {
> this._appendCubicBezier(token.text, options);
> } else if (Services.prefs.getBoolPref(CSS_GRID_ENABLED_PREF) &&
> + options.expectDisplay &&
Seems like this is complaining about http://eslint.org/docs/rules/complexity. I think similar to what we have done in other token text comparisons - we should define GRID_KEYWORDS in devtools/shared/css/properties-db and check GRID_KEYWORDS.includes(token.text). I think we can also remove CSS_GRID_ENABLED_PREF and Services since we know it is enabled at this point.
Attachment #8835769 -
Flags: review?(gl) → review+
|
|
||
Updated•9 years ago
|
Priority: -- → P3
|
||
Comment 4•9 years ago
|
||
(In reply to Gabriel Luong [:gl][1 biz day review guarantee] (ΦωΦ) from comment #3)
> I think we can also remove
> CSS_GRID_ENABLED_PREF and Services since we know it is enabled at this point.
If you do this then you can close bug 1328625.
Pushed by mpalmgren@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/4e52767ddba3
Provide grid highlighter button also for display:inline-grid elements. r=gl
https://hg.mozilla.org/integration/mozilla-inbound/rev/707b9322a8a5
Add a test that checks that the grid highlighter button also works for display:inline-grid elements.
|
|
Assignee | |
Comment 6•9 years ago
|
||
(In reply to Gabriel Luong [:gl][1 biz day review guarantee] (ΦωΦ) from comment #3)
> We should also add an unit test for testing the toggle of the grid
> highlighter for 'display:inline-grid'.
OK, I made a copy of that test using 'inline-grid' instead.
> Seems like this is complaining about
> http://eslint.org/docs/rules/complexity.
Yeah, I lifted that out to a helper function instead and eslint was happy again.
> I think we can also remove
> CSS_GRID_ENABLED_PREF and Services since we know it is enabled at this point.
We haven't actually released Grid yet and I think we'd want to keep the preference
for at least a cycle after that. I don't think you should remove the checks in
devtools until it's removed globally.
Flags: in-testsuite+
|
||
Comment 7•9 years ago
|
||
| bugherder | ||
https://hg.mozilla.org/mozilla-central/rev/4e52767ddba3
https://hg.mozilla.org/mozilla-central/rev/707b9322a8a5
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 54
|
|
||
Updated•9 years ago
|
Flags: qe-verify+
Verified fixed using the latest Nightly 54.0a1 (2017-02-20) on Ubuntu 16.04, Mac OS X 10.11 and Windows 10 x64.
Status: RESOLVED → VERIFIED
|
||
Updated•7 years ago
|
Product: Firefox → DevTools
You need to log in
before you can comment on or make changes to this bug.
Description
•