Closed
Bug 1337607
Opened 7 years ago
Closed 7 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•7 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•7 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•7 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•7 years ago
|
Priority: -- → P3
|
||
Comment 4•7 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•7 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•7 years ago
|
||
| bugherder | ||
https://hg.mozilla.org/mozilla-central/rev/4e52767ddba3 https://hg.mozilla.org/mozilla-central/rev/707b9322a8a5
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 54
|
|
||
Updated•7 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•6 years ago
|
Product: Firefox → DevTools
You need to log in
before you can comment on or make changes to this bug.
Description
•