1579582 - [Inactive CSS] 'grid-column' etc have an effect for abs.pos. boxes with a grid container as "containing block"

Status: VERIFIED FIXED

(DevTools :: Inspector, defect, P2)

Description

[Mats Palmgren (inactive)]

Attachment: Testcase

STR

1. load the attached testcase
2. open DevTools > Inspector
3. select the <x> element

ACTUAL RESULTS
In the Style pane, the grid-column declaration is greyed out and there is an information box that says:

grid-column has no effect on this element since it’s not a grid item. Try adding display:grid or display:inline-grid to the item’s parent. Learn more

This is incorrect. Abs.pos. descendants (not just children) that has a grid/subgrid container as their containing block are aligned to the grid using their grid-placement properties, so the properties do have an effect.
https://drafts.csswg.org/css-grid/#abspos-items

EXPECTED RESULTS
The grid-column declaration should be displayed normally.

Comment 1

[Patrick Brosset <:pbro>]

Thanks Mats for filing this. This is important to address quickly and maybe even uplift to beta to avoid cases where users might lose trust in the tool because it shows incorrect information.
Mike, could you please add this to your radar?

Comment 2

[Mike Ratcliffe [:miker] [:mratcliffe] [:mikeratcliffe]]

This applies to:

• grid-area
• grid-column
• grid-column-end
• grid-column-start
• grid-row
• grid-row-end
• grid-row-start

We can use a special case for justify-self, which is within the same validator (it has no effect on absolutely positioned elements).

Comment 3

[Mike Ratcliffe [:miker] [:mratcliffe] [:mikeratcliffe]]

Attachment: Bug 1579582 - 'grid-column' etc have an effect for abs.pos. boxes with a grid container as 'containing block' r?pbro!

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/3bfc227fb884
'grid-column' etc have an effect for abs.pos. boxes with a grid container as 'containing block' r=pbro

Comment 5

[Cristina Coroiu [:ccoroiu]]

https://hg.mozilla.org/mozilla-central/rev/3bfc227fb884

Comment 6

[Patrick Brosset <:pbro>]

Please verify this fix in nightly with the test case and steps in comment 0.
Thanks!

Comment 7

[Patrick Brosset <:pbro>]

Comment on attachment 9092636 [details]
Bug 1579582 - 'grid-column' etc have an effect for abs.pos. boxes with a grid container as 'containing block' r?pbro!

Beta/Release Uplift Approval Request

• User impact if declined: "Inactive CSS" is a new DevTools feature in 70 that provides super useful help to webdevs by telling them when certain parts of their CSS doesn't do what they thought it did.
  Unfortunately there are cases (such as this bug) where the logic is wrong and the tool tells people something is wrong when really it's right.

If we decline this uplift, we run the risk of people seeing this false warning and therefore losing trust in the tool. This is bad because they will be much less likely to use the feature after this point.

• Is this code covered by automated tests?: Yes
• Has the fix been verified in Nightly?: No
• Needs manual test from QE?: Yes
• If yes, steps to reproduce: I just added the qe-verify+ flag to get someone to verify this in nightly. The steps are in comment 0.
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): Not risky because a) it's just for devtools (general population unimpacted) and b) the fix is trivial, it's only removing a check for absolute positions. The rest of the code doesn't change.
• String changes made/needed:

Comment 8

[Camelia Badau [:cbadau], Desktop Test Engineering]

Reproduced the issue on Firefox Nightly 71.0a1 (2017-09-06).
Verified fixed with the test case and steps from Description on Windows 7 x64, Ubuntu 18.04 x64 and macOS 10.14 using latest Nightly 71.0a1 (2019-09-27).

Comment 9

[Liz Henry (:lizzard) (relman/hg->git project)]

Comment on attachment 9092636 [details]
Bug 1579582 - 'grid-column' etc have an effect for abs.pos. boxes with a grid container as 'containing block' r?pbro!

New feature for 70, fix verified in nightly.
Let's uplift for beta 11.

Comment 10

[Dorel Luca [:dluca]]

While I was importing this patch for an uplift I got a conflict. Looks like bug 1551581 is needed also.

This was the conflict I've got:

https://irccloud.mozilla.com/file/Rm3i2lvI/image.png

Comment 11

[Patrick Brosset <:pbro>]

I'll let Mike reply but, fwiw, I think we should make a specific patch for beta that doesn't have merge conflicts, instead of also pulling bug 1551581, so as to minimize the risk. Bug 1551581 is a new feature, not a bug fix.

Comment 12

[Mike Ratcliffe [:miker] [:mratcliffe] [:mikeratcliffe]]

Recreating a version of this patch specifically for Beta.

Comment 13

[Mike Ratcliffe [:miker] [:mratcliffe] [:mikeratcliffe]]

Attachment: [Rebased for beta] Bug 1579582 - 'grid-column' etc have an effect for abs.pos. boxes with a grid container as 'containing block' r?pbro!

Comment 14

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/releases/mozilla-beta/rev/b89912f52d32

Comment 15

[Camelia Badau [:cbadau], Desktop Test Engineering]

Verified fixed on Windows 7 x64, Ubuntu 18.04 x64 and macOS 10.14 using Firefox 70 Beta 12 build from treeherder (https://treeherder.mozilla.org/#/jobs?repo=mozilla-beta&revision=17b4f7b2c9697d78f98a702572864bd96001d1e0&selectedJob=269495211).