1478485 - Incorrect value for position:absolute's getComputedStyle left (for display:grid)

Status: RESOLVED FIXED

(Core :: DOM: CSS Object Model, defect, P3)

Description

[François REMY]

Attachment: testcase[grid-position-absolute-left-getcomputedstyle].html

User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/64.0.3282.140 Safari/537.36 Edge/18.17720

Steps to reproduce:

https://wptest.center/#/31dvt6


Actual results:

values returned by getComputedStyle doesn't take the grid properties into account before returing a value


Expected results:

values returned by getComputedStyle should round trip (0px)
https://github.com/w3c/csswg-drafts/issues/2913

Comment 1

[Mats Palmgren (inactive)]

The problem is that nsComputedDOMStyle::GetAbsoluteOffset:
https://searchfox.org/mozilla-central/rev/d4ef4e9747133aa2914aca2a15cf9df1e42a6aa0/layout/style/nsComputedDOMStyle.cpp#4142
assumes that the CB rect is the content rect of the container.

Comment 2

[Mats Palmgren (inactive)]

Attachment: fix

Comment 3

[Mats Palmgren (inactive)]

I was expecting a WPT unexpected-fail but I don't see it:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=3660ec79089c0a26cb74ddda2d5cb3bc8fc58043

François, do you intend to add a WPT for this?

Comment 4

[Daniel Holbert [:dholbert]]

Comment on attachment 9007896 [details] [diff] [review]
fix

Review of attachment 9007896 [details] [diff] [review]:
-----------------------------------------------------------------

r=me with nit addressed (assuming this ends up with a test here, or in WPT pretty soon)

::: layout/style/nsComputedDOMStyle.cpp
@@ +4164,5 @@
>      }
> +  } else if (container->IsGridContainerFrame() &&
> +             (mOuterFrame->HasAnyStateBits(NS_FRAME_OUT_OF_FLOW))) {
> +    auto gridContainer = static_cast<nsGridContainerFrame*>(container);
> +    containerRect = gridContainer->GridItemCB(mOuterFrame);

GridItemCB() is a static function on the nsGridContainerFrame class, so you shouldn't call it on the gridContainer instance.

So I think we can just collapse these 2 lines to just the following:
 containerRect = nsGridContainerFrame::GridItemCB((mOuterFrame);

See for reference:
https://dxr.mozilla.org/mozilla-central/rev/9bb0f7fc73ad2109e4ea777e9ba65a16a6acc9cd/layout/generic/nsAbsoluteContainingBlock.cpp#136-137

You also might want to update the documentation at https://dxr.mozilla.org/mozilla-central/rev/9bb0f7fc73ad2109e4ea777e9ba65a16a6acc9cd/layout/generic/nsGridContainerFrame.h#162-163 which currently indicates that it's "not meant to be used elsewhere" (not for use outside of nsAbsoluteContainingBlock).

Comment 5

[Mats Palmgren (inactive)]

Good points, thanks.

(Also, I meant s/unexpected-fail/unexpected-pass/ of course in my comment 3)

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/9a89fcb3c96c
Use the stored CB on abs.pos. grid items when calculating used offset values.  r=dholbert

Comment 7

[Cosmin Sabou [:CosminS]]

https://hg.mozilla.org/mozilla-central/rev/9a89fcb3c96c