Closed Bug 1595256 Opened 27 days ago Closed 16 days ago

[Inactive CSS] Inspector says `grid-gap` is inactive for flexbox, but it's not

Categories

(DevTools :: Inspector: Rules, defect, P2)

defect

Tracking

(firefox72 fixed)

RESOLVED FIXED
Firefox 72
Tracking Status
firefox72 --- fixed

People

(Reporter: birtles, Assigned: tony)

References

(Blocks 1 open bug)

Details

(Keywords: good-first-bug, Whiteboard: [lang=js])

Attachments

(3 files)

Attached file Test case

STR:

  1. Create an element with display: flex.
  2. Specify grid-gap: 5em on it.
  3. Inspect it.
    ... Or just open the attached file and inspect the <ul> element.

Expected results:
The grid-gap rule is shown normally.

Actual results:
The grip-gap rule is greyed out and the ℹ️ icon indicates that it has no effect. But it does. Disable the rule and you will see.

Oh wow, ok, grid-gap works with flexbox. I had no idea. Thanks for filing this issue Brian.

I think the fix for it would be to move this line: https://searchfox.org/mozilla-central/rev/cac340f10816707e91c46db6d285f80259420f07/devtools/server/actors/utils/inactive-property-helper.js#103

To be inside this block instead: https://searchfox.org/mozilla-central/rev/cac340f10816707e91c46db6d285f80259420f07/devtools/server/actors/utils/inactive-property-helper.js#138-145

Anyone interested in fixing this is more than welcome to do so. If this is your first contribution, please make sure to go through the DevTools contribution docs first: https://docs.firefox-dev.tools/

Blocks: 1540753
Keywords: good-first-bug
Priority: -- → P2
Whiteboard: [lang=js]

Hi there, this will be my first time following the proper procedure in contributing for Mozilla, and this looks like a good place for me to start. Do you mind if I take the assignment for this bug?

(In reply to Patrick Brosset <:pbro> from comment #1)

Oh wow, ok, grid-gap works with flexbox. I had no idea.

All the grid‑*‑gap properties are specified and implemented as legacy shorthands for the equivalent *‑gap properties: https://drafts.csswg.org/css-align-3/#gap-legacy

Thank you Tony for working on this. I just gave a couple comments in phabricator.
Thank you ExE Boss for the link to the spec, that helps understand better!

Assignee: nobody → tony
Status: NEW → ASSIGNED

Revision submitted, but I made a mistake with my working directory and Mercurial, resulting in a new phabricator revision. My bad Patrick, sorry!

Attachment #9109850 - Attachment description: Bug 1595256 - Addressing revision issues: amended grid-*-gap legacy shorthands to invalid properties of non-flex, non-grid, or non-multi-col containers. r=pbro! → Bug 1595256 - Amended grid-*-gap legacy shorthands to invalid properties of non-flex, non-grid, or non-multi-col containers. r=pbro!
Pushed by pbrosset@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/a608016e1c34
Amended grid-*-gap legacy shorthands to invalid properties of non-flex, non-grid, or non-multi-col containers. r=pbro
Pushed by rmaries@mozilla.com:
https://hg.mozilla.org/mozilla-central/rev/098c0e395cc3
Amended grid-*-gap legacy shorthands to invalid properties of non-flex, non-grid, or non-multi-col containers. r=pbro
Status: ASSIGNED → RESOLVED
Closed: 16 days ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 72

Since grid‑gap is an alias for gap, it too is supported in the multi‑column context.

You need to log in before you can comment on or make changes to this bug.