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

Status: RESOLVED FIXED

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

Description

[Brian Birtles (:birtles)]

Attachment: 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.

Comment 1

[Patrick Brosset <:pbro>]

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/

Comment 2

[Tony Wang]

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?

Comment 3

[Tony Wang]

Attachment: Bug 1595256 - Moved grid-gap invalid property to not-flex-container invalid properties. r=pbro!

Comment 4

[ExE Boss]

(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

Comment 5

[Patrick Brosset <:pbro>]

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!

Comment 6

[Tony Wang]

Attachment: Bug 1595256 - Amended grid-*-gap legacy shorthands to invalid properties of non-flex, non-grid, or non-multi-col containers. r=pbro!

Comment 7

[Tony Wang]

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

Comment 8

[Pulsebot]

Pushed by [e-mail]:
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

Comment 9

[Pulsebot]

Pushed by [e-mail]:
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

Comment 10

[Razvan Maries]

https://hg.mozilla.org/mozilla-central/rev/098c0e395cc3

Comment 11

[ExE Boss]

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