New issue
Advanced search Search tips

Issue 628155 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner: ----
Closed: Sep 2016
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 3
Type: Bug

Blocking:
issue 79180



Sign in to add a comment

[css-grid] Items overflowing the grid area due to margins are not painted in some situations

Project Member Reported by r...@igalia.com, Jul 14 2016

Issue description


The issue is in LayoutGrid::layoutGridItems(), when we keep track of the items overflowing the grid area [1].
        // Keep track of children overflowing their grid area as we might need to paint them even if the grid-area is
        // not visible
        if (child->logicalHeight() > overrideContainingBlockContentLogicalHeight
            || child->logicalWidth() > overrideContainingBlockContentLogicalWidth)
            m_gridItemsOverflowingGridArea.append(child);
    }

We only take into account the height or the width, but not the the margins.

In the attached example (check it live at [2]) there's a grid with 4 columns: 100px 50px 150px 100px;
There's a red item in the 2nd column.
Then there's a grid item in the 4th column, with "margin-left: -200px;" and a fixed width of 50px. The item should appear just on top of the red item, because of the margin.
The grid is embeded in an iframe of 300px width, so without scroll you can only see the first 3 columns.
As we only check the size of the green item to determine if it's overflowing or not, we decide it's not (50px < 100px). And like we don't paint the forth column, we don't paint that item either, which is wrong.

We should check margins too to avoid this issue. This was detected by @cbiesinger in an unrelated patch review that was touching this code [3].

[1] https://chromium.googlesource.com/chromium/src.git/+/f7dbf39be31d8aa9214d5d84da613508d4e06491/third_party/WebKit/Source/core/layout/LayoutGrid.cpp#1720
[2] http://jsbin.com/fazowo/1/edit?html,css,output
[3] https://codereview.chromium.org/842193004/#msg9
 
bug-painting-item-overlfowing-margins.html
382 bytes View Download
bug-painting-item-overlfowing-margins-current.png
680 bytes View Download
bug-painting-item-overlfowing-margins-expected.png
669 bytes View Download
Project Member

Comment 1 by bugdroid1@chromium.org, Sep 21 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/a67debf9a9b78f60e7b10b5574ae4ceab49af31b

commit a67debf9a9b78f60e7b10b5574ae4ceab49af31b
Author: svillar <svillar@igalia.com>
Date: Wed Sep 21 17:29:37 2016

[css-grid] Use grid area position to determine overflowing items

Grid code keeps a list of items overflowing their grid areas so that it
could apply some painting optimizations to reduce the amount of grid items
to be painted. Even if the border box is completely contained by the
item's grid area, margins in the item could make it overflow its grid area
by altering the border box position.

The problem is not that the optimization won't work, it's worse because it will
make that item not visible as it won't be painted.

BUG= 628155 

Review-Url: https://codereview.chromium.org/2353513003
Cr-Commit-Position: refs/heads/master@{#420094}

[add] https://crrev.com/a67debf9a9b78f60e7b10b5574ae4ceab49af31b/third_party/WebKit/LayoutTests/fast/css-grid-layout/painting-item-marginbox-overflowing-grid-area-expected.html
[add] https://crrev.com/a67debf9a9b78f60e7b10b5574ae4ceab49af31b/third_party/WebKit/LayoutTests/fast/css-grid-layout/painting-item-marginbox-overflowing-grid-area.html
[modify] https://crrev.com/a67debf9a9b78f60e7b10b5574ae4ceab49af31b/third_party/WebKit/Source/core/layout/LayoutGrid.cpp
[modify] https://crrev.com/a67debf9a9b78f60e7b10b5574ae4ceab49af31b/third_party/WebKit/Source/core/layout/LayoutGrid.h

Comment 2 by svil...@igalia.com, Sep 23 2016

Status: Fixed (was: Available)
Closing

Sign in to add a comment