New issue
Advanced search Search tips

Issue 680468 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner: ----
Closed: Jul 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug

Blocking:
issue 273898



Sign in to add a comment

[css-grid] Dynamically setting "position: absolute" in a grid item doesn't trigger a relayout of that element

Project Member Reported by r...@igalia.com, Jan 12 2017

Issue description


Check the attached example, you shouldn't see red as the item gets absolute positioned and should take the whole size of the grid.

Note that in the example, when "item" is marked as absolutely positioned, its containing block is "wrapper" and not the grid container.
For some reason we're not triggering a new layout of the "item" in this situation.
 
test-positioned-relayout.html
500 bytes View Download

Comment 1 by r...@igalia.com, Jan 12 2017

Summary: [css-grid] Dynamically setting "position: absolute" in a grid item doesn't trigger a relayout of that element (was: [css-grid] Dynamically setting "position: absolute" in a grid item doesn't trigger a relayout)

Comment 2 by r...@igalia.com, Jan 12 2017

Description: Show this description

Comment 3 by r...@igalia.com, Jan 13 2017

Cc: cbiesin...@chromium.org
This issue is not reproducible in Flexbox, after investigating it for a while
it seems than in LayoutBlock::layoutPositionedObjects() we're lacking a similar code for Grid:
https://cs.chromium.org/chromium/src/third_party/WebKit/Source/core/layout/LayoutBlock.cpp?cl=GROK&gsn=layoutPositionedObjects&l=817

    LayoutObject* parent = positionedObject->parent();
    bool layoutChanged = false;
    if (parent->isFlexibleBox() &&
        toLayoutFlexibleBox(parent)->setStaticPositionForPositionedLayout(
            *positionedObject)) {
      // The static position of an abspos child of a flexbox depends on its size
      // (for example, they can be centered). So we may have to reposition the
      // item after layout.
      // TODO(cbiesinger): We could probably avoid a layout here and just
      // reposition?
      positionedObject->forceChildLayout();
      layoutChanged = true;
    }

In the example, when we mark the "item" as "position: absolute"
layoutPositionedObjects() on the wrapper is called,
however this is not forcing a layout of the "item".

Comment 4 by r...@igalia.com, Jan 13 2017

Oops, it seems I was too fast as I cannot reproduce it in flexbox and I found that code I thought it was related,
but that forceChildLayout() is not called in this flexbox example,
and calling it for grid doesn't solve the issue.

It seems the "item" is properly marked as needsLayout() in:
    if (!positionedObject->normalChildNeedsLayout() &&
        (relayoutChildren || m_heightAvailableToChildrenChanged ||
         needsLayoutDueToStaticPosition(positionedObject))) {
      layoutScope.setChildNeedsLayout(positionedObject);
    }

So this needs deeper investigation. :-/

I'm attaching the flexbox example just for reference, as it works properly there.
test-positioned-relayout-flex.html
510 bytes View Download

Comment 5 by svil...@igalia.com, Jul 4 2017

Status: Assigned (was: Available)
I'm working on this. The problem is not the lack of layouts but the fact that we have previously set an override size. Forcing a layout in those cases do not help as the containing block override size would be the same.
Project Member

Comment 6 by bugdroid1@chromium.org, Jul 6 2017

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

commit 2a504cc9bdf1418fc0cb67923c1c1e93c78c857e
Author: Sergio Villar Senin <svillar@igalia.com>
Date: Thu Jul 06 16:20:19 2017

Containing block overrides not cleared for position:absolute

Whenever a position:absolute block gets a new containing block the
previously set containing block overrides are not cleared. This causes the
block not to be properly layout for its new containing block (for example
when using relative sizes).

In particular this affects grid items which always get a containing block
override size (which represent the grid areas) in case their
containing block switches from the grid container to a grid ancestor.

Bug:  680468 
Change-Id: I27f2cf871ba011db3f9532368b22a038b7a4c5b3
Reviewed-on: https://chromium-review.googlesource.com/559547
Reviewed-by: Christian Biesinger <cbiesinger@chromium.org>
Commit-Queue: Sergio Villar <svillar@igalia.com>
Cr-Commit-Position: refs/heads/master@{#484620}
[add] https://crrev.com/2a504cc9bdf1418fc0cb67923c1c1e93c78c857e/third_party/WebKit/LayoutTests/fast/css-grid-layout/absolute-positioning-changing-containing-block-expected.html
[add] https://crrev.com/2a504cc9bdf1418fc0cb67923c1c1e93c78c857e/third_party/WebKit/LayoutTests/fast/css-grid-layout/absolute-positioning-changing-containing-block.html
[add] https://crrev.com/2a504cc9bdf1418fc0cb67923c1c1e93c78c857e/third_party/WebKit/LayoutTests/fast/css-grid-layout/grid-item-absolute-positioning-dynamic-expected.html
[add] https://crrev.com/2a504cc9bdf1418fc0cb67923c1c1e93c78c857e/third_party/WebKit/LayoutTests/fast/css-grid-layout/grid-item-absolute-positioning-dynamic.html
[modify] https://crrev.com/2a504cc9bdf1418fc0cb67923c1c1e93c78c857e/third_party/WebKit/Source/core/layout/LayoutBlock.cpp

Comment 7 by svil...@igalia.com, Jul 7 2017

Status: Fixed (was: Assigned)
This should be fixed now, please verify.

Sign in to add a comment