Skip to content

Commit

Permalink
Cherry-pick ff409be. rdar://problem/109170495
Browse files Browse the repository at this point in the history
    [css-grid][masonry] Containing block for items in a masonry columns grid should be the grid's content box logical width
    https://bugs.webkit.org/show_bug.cgi?id=256611
    rdar://109170495

    Reviewed by Matt Woodrow.

    The masonry spec states, "The containing block for a grid item is
    formed by its grid area in the grid axis and the grid container’s
    content-box in the masonry axis."

    This means that for a grid that has masonry columns specified, the
    masonry axis will be in the logical width direction of the grid. The
    items should have their containing block set to the content box logical
    box of the grid.

    We can modify GridTrackSizingAlgorithm::gridAreaBreadthForChild to
    return this value when the grid is a masonry columns grid since normally
    the grid area would be used as the containing block in non-masonry
    grids. This allows the rest of the code to use the grid area sizes of
    the containing block in both masonry and non-masonry scenarios.
    Previously, this function would have attempted to compute the
    value by iterating over the tracks in the specified direction, but there
    are no tracks in the masonry direction so we use the logic specified by
    the masonry spec instead.

    However, in order for this change to work properly we had to make a
    change in RenderGrid::layoutMasonry by removing code that was
    incorrectly overriding the logical width of the grid. The previous code
    was attempting to set the logical width of the grid to the masonry
    content size when the grid had masonry columns specified and an auto
    logical width. There were 2 main issues with this piece of code:
    1.) m_masonryLayout.gridContentSize() will always return 0 since we
    actually haven't performed masonry layout at this point
    2.) The grid shouldn't be overriding its logical width like this anyways
    and it should instead be set by sized by the rules of the formatting
    context it is participating in (e.g. block or inline layout).

    By removing this code we can get the actual width of the grid later on
    when we call m_renderGrid->contentLogicalWidth() rather than the
    incorrect 0 value that it was being set to.

    The following example highlights the changes that were made.
    <style>
    grid {
        display: grid;
        grid-template-columns:masonry;
        grid-template-rows: auto;
    }
    </style>
    <grid>
        <svg width="100" height="100" viewBox="0 0 1 1" style="width: 100%; max-width: 100px; background: green;"></svg>
    </grid>

    By removing the extra code in RenderGrid::layoutMasonry, the grid will
    get sizes as a block level box in the block formatting context it is
    participating in, giving it a logical width that takes up its available
    space. The svg's containing block logical width is set to its value so
    it is able to resolve its percentage width to the correct value whereas
    before the containing block logical width would have been 0px.

    https://drafts.csswg.org/css-grid-3/#containing-block

    * LayoutTests/TestExpectations:
    * LayoutTests/imported/w3c/web-platform-tests/css/css-grid/masonry/tentative/masonry-columns-item-containing-block-is-grid-content-width-expected.html: Added.
    * LayoutTests/imported/w3c/web-platform-tests/css/css-grid/masonry/tentative/masonry-columns-item-containing-block-is-grid-content-width.html: Added.
    * LayoutTests/platform/glib/TestExpectations:
    * Source/WebCore/rendering/GridTrackSizingAlgorithm.cpp:
    (WebCore::GridTrackSizingAlgorithm::gridAreaBreadthForChild const):
    * Source/WebCore/rendering/GridTrackSizingAlgorithm.h:
    * Source/WebCore/rendering/RenderGrid.cpp:
    (WebCore::RenderGrid::layoutMasonry):

    Canonical link: https://commits.webkit.org/264011@main

Identifier: 263769.30@safari-7616.1.14.10-branch
  • Loading branch information
sammygill authored and MyahCobbs committed May 15, 2023
1 parent 47212f0 commit eca1876
Show file tree
Hide file tree
Showing 7 changed files with 29 additions and 6 deletions.
1 change: 0 additions & 1 deletion LayoutTests/TestExpectations
Original file line number Diff line number Diff line change
Expand Up @@ -1581,7 +1581,6 @@ imported/w3c/web-platform-tests/css/css-grid/masonry/tentative/align-tracks/maso
imported/w3c/web-platform-tests/css/css-grid/masonry/tentative/align-tracks/masonry-align-tracks-stretch-002.html
imported/w3c/web-platform-tests/css/css-grid/masonry/tentative/fragmentation/masonry-fragmentation-001.html [ ImageOnlyFailure ]
imported/w3c/web-platform-tests/css/css-grid/masonry/tentative/fragmentation/masonry-fragmentation-002.html [ ImageOnlyFailure ]
imported/w3c/web-platform-tests/css/css-grid/masonry/tentative/fragmentation/masonry-fragmentation-003.html [ ImageOnlyFailure ]
imported/w3c/web-platform-tests/css/css-grid/masonry/tentative/fragmentation/masonry-fragmentation-004.html [ ImageOnlyFailure ]
imported/w3c/web-platform-tests/css/css-grid/masonry/tentative/fragmentation/masonry-fragmentation-005.html [ ImageOnlyFailure ]
imported/w3c/web-platform-tests/css/css-grid/masonry/tentative/fragmentation/masonry-fragmentation-006.html [ ImageOnlyFailure ]
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
<!DOCTYPE html>
<p>Test passes if there is a filled green square.</p>
<div style="width:100px; height:100px; background:green;"></div>
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
<!DOCTYPE html>
<html>
<head>
<link rel="author" title="Sammy Gill" href="mailto:sammy.gill@apple.com">
<link rel="help" href="https://drafts.csswg.org/css-grid-3/#containing-block">
<link rel="match" href="/css/reference/ref-filled-green-100px-square-only.html">
<meta name="assert" content="Svg should use grid's content logical width for its containing block size and get sized to 100px x 100px">
<style>
grid {
display: grid;
grid-template-columns:masonry;
grid-template-rows: auto;
}
</style>
</head>
<body>
<p>Test passes if there is a filled green square.</p>
<grid>
<svg width="100" height="100" viewBox="0 0 1 1" style="width: 100%; max-width: 100px; background: green;"></svg>
</grid>
</body>
</html>
2 changes: 0 additions & 2 deletions LayoutTests/platform/glib/TestExpectations
Original file line number Diff line number Diff line change
Expand Up @@ -340,8 +340,6 @@ imported/mozilla/svg/text/textpath-multiline.svg [ Pass ]
imported/w3c/web-platform-tests/css/css-align/baseline-rules/grid-item-input-type-number.html [ Pass ]
imported/w3c/web-platform-tests/css/css-align/baseline-rules/grid-item-input-type-text.html [ Pass ]

imported/w3c/web-platform-tests/css/css-grid/masonry/tentative/align-content/masonry-align-content-004.html [ Pass ]

imported/w3c/web-platform-tests/css/css-images/image-orientation/image-orientation-background-properties.html [ Pass ]
imported/w3c/web-platform-tests/css/css-images/image-orientation/image-orientation-default.html [ Pass ]
imported/w3c/web-platform-tests/css/css-images/image-orientation/image-orientation-from-image-composited-dynamic1.html [ Pass ]
Expand Down
3 changes: 3 additions & 0 deletions Source/WebCore/rendering/GridTrackSizingAlgorithm.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -637,6 +637,9 @@ std::optional<LayoutUnit> GridTrackSizingAlgorithm::estimatedGridAreaBreadthForC

std::optional<LayoutUnit> GridTrackSizingAlgorithm::gridAreaBreadthForChild(const RenderBox& child, GridTrackSizingDirection direction) const
{
if (m_renderGrid->areMasonryColumns())
return m_renderGrid->contentLogicalWidth();

bool addContentAlignmentOffset =
direction == ForColumns && (m_sizingState == RowSizingFirstIteration || m_sizingState == RowSizingExtraIterationForSizeContainment);
// To determine the column track's size based on an orthogonal grid item we need it's logical
Expand Down
1 change: 1 addition & 0 deletions Source/WebCore/rendering/GridTrackSizingAlgorithm.h
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
#include "GridBaselineAlignment.h"
#include "GridTrackSize.h"
#include "LayoutSize.h"
#include "RenderBoxInlines.h"

namespace WebCore {

Expand Down
3 changes: 0 additions & 3 deletions Source/WebCore/rendering/RenderGrid.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -426,9 +426,6 @@ void RenderGrid::layoutMasonry(bool relayoutChildren)

LayoutUnit availableSpaceForColumns = availableLogicalWidth();
placeItemsOnGrid(availableSpaceForColumns);
// Size in the masonry axis is the masonry content size
if (areMasonryColumns() && style().logicalWidth().isAuto())
setLogicalWidth(m_masonryLayout.gridContentSize() + borderAndPaddingLogicalWidth());

m_trackSizingAlgorithm.setAvailableSpace(ForColumns, availableSpaceForColumns);
performGridItemsPreLayout(m_trackSizingAlgorithm);
Expand Down

0 comments on commit eca1876

Please sign in to comment.