Bug 193657 - [css-grid] Issue with abspos element which containing block is the grid container
Summary: [css-grid] Issue with abspos element which containing block is the grid conta...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: CSS (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Oriol Brufau
URL:
Keywords: InRadar
Depends on: 193736
Blocks:
  Show dependency treegraph
 
Reported: 2019-01-21 12:01 PST by Oriol Brufau
Modified: 2019-01-23 14:29 PST (History)
6 users (show)

See Also:


Attachments
testcase (369 bytes, text/html)
2019-01-21 12:01 PST, Oriol Brufau
no flags Details
Patch (825.36 KB, patch)
2019-01-21 12:16 PST, Oriol Brufau
no flags Details | Formatted Diff | Diff
Patch (825.33 KB, patch)
2019-01-21 13:27 PST, Oriol Brufau
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Oriol Brufau 2019-01-21 12:01:12 PST
Created attachment 359702 [details]
testcase

See the attached example, the abspos element is a child of the grid item.
The containing block is the grid container.

If you set the offset in just one axis (e.g. "left: 0"), the abspos element
is moved to the left but also to the top (losing its static position
in the vertical axis), which shouldn't happen.

This works fine in Firefox, we should manage this per axis (inline or block)
independently.

Has been fixed in Chromium: https://crbug.com/828628
Comment 1 Oriol Brufau 2019-01-21 12:16:13 PST
Created attachment 359703 [details]
Patch
Comment 2 Javier Fernandez 2019-01-21 13:01:03 PST
Comment on attachment 359703 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=359703&action=review

> Source/WebCore/rendering/RenderGrid.cpp:1760
> +// TODO: SetLogicalPositionForChild has only one caller, consider its

We use FIXME for this type of comments. Also, there is no need to break the line.

> Source/WebCore/rendering/RenderGrid.cpp:1764
> +    // "In the positioning phase [...] calculations are performed according to the

We can use longer lines and avoid such large number of lines in this comment.

> Source/WebCore/rendering/RenderGrid.cpp:1776
> +    // 'setLogicalLeft' and 'setLogicalTop' only take into account the child's

Ditto
Comment 3 Javier Fernandez 2019-01-21 13:02:46 PST
Comment on attachment 359703 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=359703&action=review

> Source/WebCore/rendering/RenderGrid.cpp:1552
> +    ASSERT(child.IsOutOfFlowPositioned());

It should be "isOutOfFlowPositioned()"
Comment 4 Oriol Brufau 2019-01-21 13:27:39 PST
Created attachment 359707 [details]
Patch
Comment 5 WebKit Commit Bot 2019-01-23 04:45:02 PST
Comment on attachment 359707 [details]
Patch

Clearing flags on attachment: 359707

Committed r240333: <https://trac.webkit.org/changeset/240333>
Comment 6 WebKit Commit Bot 2019-01-23 04:45:03 PST
All reviewed patches have been landed.  Closing bug.
Comment 7 Radar WebKit Bug Importer 2019-01-23 04:46:29 PST
<rdar://problem/47478277>
Comment 8 Truitt Savell 2019-01-23 08:16:21 PST
two of the imported/w3c tests are failing after their introduction in this patch https://trac.webkit.org/changeset/240333/webkit on Mac.

imported/w3c/web-platform-tests/css/css-grid/abspos/orthogonal-positioned-grid-descendants-010.html
imported/w3c/web-platform-tests/css/css-grid/abspos/orthogonal-positioned-grid-descendants-013.html

History:
http://webkit-test-results.webkit.org/dashboards/flakiness_dashboard.html#showAllRuns=true&tests=imported%2Fw3c%2Fweb-platform-tests%2Fcss%2Fcss-grid%2Fabspos%2Forthogonal-positioned-grid-descendants-010.html%20imported%2Fw3c%2Fweb-platform-tests%2Fcss%2Fcss-grid%2Fabspos%2Forthogonal-positioned-grid-descendants-013.html

This is making the tree red and could effect EWS and must be resolved.
Comment 9 Ryan Haddad 2019-01-23 14:29:39 PST
(In reply to Truitt Savell from comment #8)
> two of the imported/w3c tests are failing after their introduction in this
> patch https://trac.webkit.org/changeset/240333/webkit on Mac.
> 
> imported/w3c/web-platform-tests/css/css-grid/abspos/orthogonal-positioned-
> grid-descendants-010.html
> imported/w3c/web-platform-tests/css/css-grid/abspos/orthogonal-positioned-
> grid-descendants-013.html
> 
> History:
> http://webkit-test-results.webkit.org/dashboards/flakiness_dashboard.
> html#showAllRuns=true&tests=imported%2Fw3c%2Fweb-platform-tests%2Fcss%2Fcss-
> grid%2Fabspos%2Forthogonal-positioned-grid-descendants-010.
> html%20imported%2Fw3c%2Fweb-platform-tests%2Fcss%2Fcss-
> grid%2Fabspos%2Forthogonal-positioned-grid-descendants-013.html
> 
> This is making the tree red and could effect EWS and must be resolved.

Oriol marked these tests as flaky in https://trac.webkit.org/changeset/240362