New issue
Advanced search Search tips

Issue 750681 link

Starred by 1 user
Status: Fixed
Owner:
jfernan...@igalia.com
Closed: Sep 2017
Cc:
jfernan...@igalia.com
r...@igalia.com
svil...@igalia.com
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

[css-grid] Changing alignment from stretch to normal doesn't resize an image

Project Member Reported by r...@igalia.com, Jul 31 2017 

Check the attached example, on the first layout the image is 200x100
(because of the "stretch" alignment).
Then alignment is changed from "stretch" to "normal" and the image is not resized.
It should be 200x200, but it's still 200x100.
stretch-to-normal.html
342 bytes View Download
stretch-to-normal-current.png
5.2 KB View Download
stretch-to-normal-expected.png
5.9 KB View Download

Comment 1 by jfernan...@igalia.com, Jul 31 2017 

This issue looks like a regression. Could you please bisect it ?

Comment 2 by jfernan...@igalia.com, Jul 31 2017 

I suspect r475400 may be the culprit, but could you please verify it ? Thanks.

Comment 3 by r...@igalia.com, Aug 3 2017 

@jfernandez I've bisected it and it points to a different commit r478722.

I guess that the problem is that for regular items a change between "normal" and "stretch"
shouldn't generate a new relayout as they behave the same.
But for replaced elements, they're different.

Comment 4 by jfernan...@igalia.com, Aug 11 2017

Status: Started (was: Available)
I'm working now on this issue.

Comment 5 by jfernan...@igalia.com, Aug 14 2017

Owner: jfernan...@igalia.com
Project Member

Comment 6 by bugdroid1@chromium.org, Sep 2 2017 

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

commit ca421e607aecefc211d829c996e21106190a1ff5
Author: Javier Fernandez <jfernandez@igalia.com>
Date: Sat Sep 02 00:50:35 2017

[css-grid] Don't resolve Default Alignment 'normal 'values

Our logic to detect style changes that may affect the grid item's size
checks whether the value has changed from or to stretch. In case of any
of the involved values is 'normal', we are using the grid's 'normal'
behavior, which as per specs it'll be resolved as 'stretch'.

However, in case of replaced items we must use resolve any 'normal'
value as 'start', instead of 'stretch', so it uses its intrinsic size.

Hence, we can't assume Default Alignment 'normal' values can be resolved
to 'stretch' in all the cases; we will need to evaluate each grid item's
nature (obviously if its Self Alignment value is 'auto').

Bug:  750681 
Change-Id: I869848904c75402b6ab247509090a86bac4030a1
Reviewed-on: https://chromium-review.googlesource.com/611802
Commit-Queue: Javier Fernandez <jfernandez@igalia.com>
Reviewed-by: Manuel Rego Casasnovas <rego@igalia.com>
Cr-Commit-Position: refs/heads/master@{#499358}
[add] https://crrev.com/ca421e607aecefc211d829c996e21106190a1ff5/third_party/WebKit/LayoutTests/external/wpt/css/css-grid-1/alignment/grid-alignment-implies-size-change-001.html
[add] https://crrev.com/ca421e607aecefc211d829c996e21106190a1ff5/third_party/WebKit/LayoutTests/external/wpt/css/css-grid-1/alignment/grid-alignment-implies-size-change-002.html
[add] https://crrev.com/ca421e607aecefc211d829c996e21106190a1ff5/third_party/WebKit/LayoutTests/external/wpt/css/css-grid-1/alignment/grid-alignment-implies-size-change-003.html
[add] https://crrev.com/ca421e607aecefc211d829c996e21106190a1ff5/third_party/WebKit/LayoutTests/external/wpt/css/css-grid-1/alignment/grid-alignment-implies-size-change-004.html
[add] https://crrev.com/ca421e607aecefc211d829c996e21106190a1ff5/third_party/WebKit/LayoutTests/external/wpt/css/css-grid-1/alignment/grid-alignment-implies-size-change-005.html
[add] https://crrev.com/ca421e607aecefc211d829c996e21106190a1ff5/third_party/WebKit/LayoutTests/external/wpt/css/css-grid-1/alignment/grid-alignment-implies-size-change-006.html
[add] https://crrev.com/ca421e607aecefc211d829c996e21106190a1ff5/third_party/WebKit/LayoutTests/external/wpt/css/css-grid-1/alignment/grid-alignment-implies-size-change-007.html
[add] https://crrev.com/ca421e607aecefc211d829c996e21106190a1ff5/third_party/WebKit/LayoutTests/external/wpt/css/css-grid-1/alignment/grid-alignment-implies-size-change-008.html
[add] https://crrev.com/ca421e607aecefc211d829c996e21106190a1ff5/third_party/WebKit/LayoutTests/external/wpt/css/css-grid-1/alignment/grid-alignment-implies-size-change-009.html
[add] https://crrev.com/ca421e607aecefc211d829c996e21106190a1ff5/third_party/WebKit/LayoutTests/external/wpt/css/css-grid-1/alignment/grid-alignment-implies-size-change-010.html
[add] https://crrev.com/ca421e607aecefc211d829c996e21106190a1ff5/third_party/WebKit/LayoutTests/external/wpt/css/css-grid-1/alignment/grid-alignment-implies-size-change-011.html
[add] https://crrev.com/ca421e607aecefc211d829c996e21106190a1ff5/third_party/WebKit/LayoutTests/external/wpt/css/css-grid-1/alignment/grid-alignment-implies-size-change-012.html
[add] https://crrev.com/ca421e607aecefc211d829c996e21106190a1ff5/third_party/WebKit/LayoutTests/external/wpt/css/css-grid-1/alignment/grid-alignment-implies-size-change-013.html
[add] https://crrev.com/ca421e607aecefc211d829c996e21106190a1ff5/third_party/WebKit/LayoutTests/external/wpt/css/css-grid-1/alignment/grid-alignment-implies-size-change-014.html
[add] https://crrev.com/ca421e607aecefc211d829c996e21106190a1ff5/third_party/WebKit/LayoutTests/external/wpt/css/css-grid-1/alignment/grid-alignment-implies-size-change-015.html
[add] https://crrev.com/ca421e607aecefc211d829c996e21106190a1ff5/third_party/WebKit/LayoutTests/external/wpt/css/css-grid-1/alignment/grid-alignment-implies-size-change-016.html
[add] https://crrev.com/ca421e607aecefc211d829c996e21106190a1ff5/third_party/WebKit/LayoutTests/external/wpt/css/css-grid-1/alignment/grid-alignment-implies-size-change-017.html
[add] https://crrev.com/ca421e607aecefc211d829c996e21106190a1ff5/third_party/WebKit/LayoutTests/external/wpt/css/css-grid-1/alignment/grid-alignment-implies-size-change-018.html
[add] https://crrev.com/ca421e607aecefc211d829c996e21106190a1ff5/third_party/WebKit/LayoutTests/external/wpt/css/css-grid-1/alignment/grid-alignment-implies-size-change-019.html
[add] https://crrev.com/ca421e607aecefc211d829c996e21106190a1ff5/third_party/WebKit/LayoutTests/external/wpt/css/css-grid-1/alignment/grid-alignment-implies-size-change-020.html
[add] https://crrev.com/ca421e607aecefc211d829c996e21106190a1ff5/third_party/WebKit/LayoutTests/external/wpt/css/css-grid-1/alignment/grid-alignment-implies-size-change-021.html
[add] https://crrev.com/ca421e607aecefc211d829c996e21106190a1ff5/third_party/WebKit/LayoutTests/external/wpt/css/css-grid-1/alignment/grid-alignment-implies-size-change-022.html
[add] https://crrev.com/ca421e607aecefc211d829c996e21106190a1ff5/third_party/WebKit/LayoutTests/external/wpt/css/css-grid-1/alignment/grid-alignment-implies-size-change-023.html
[add] https://crrev.com/ca421e607aecefc211d829c996e21106190a1ff5/third_party/WebKit/LayoutTests/external/wpt/css/css-grid-1/alignment/grid-alignment-implies-size-change-024.html
[add] https://crrev.com/ca421e607aecefc211d829c996e21106190a1ff5/third_party/WebKit/LayoutTests/external/wpt/css/css-grid-1/alignment/grid-alignment-implies-size-change-025.html
[add] https://crrev.com/ca421e607aecefc211d829c996e21106190a1ff5/third_party/WebKit/LayoutTests/external/wpt/css/css-grid-1/alignment/grid-alignment-implies-size-change-026.html
[add] https://crrev.com/ca421e607aecefc211d829c996e21106190a1ff5/third_party/WebKit/LayoutTests/external/wpt/css/css-grid-1/alignment/grid-alignment-implies-size-change-027.html
[add] https://crrev.com/ca421e607aecefc211d829c996e21106190a1ff5/third_party/WebKit/LayoutTests/external/wpt/css/css-grid-1/alignment/grid-alignment-implies-size-change-028.html
[add] https://crrev.com/ca421e607aecefc211d829c996e21106190a1ff5/third_party/WebKit/LayoutTests/external/wpt/css/css-grid-1/alignment/grid-alignment-implies-size-change-029.html
[add] https://crrev.com/ca421e607aecefc211d829c996e21106190a1ff5/third_party/WebKit/LayoutTests/external/wpt/css/css-grid-1/alignment/grid-alignment-implies-size-change-030.html
[add] https://crrev.com/ca421e607aecefc211d829c996e21106190a1ff5/third_party/WebKit/LayoutTests/external/wpt/css/css-grid-1/alignment/grid-alignment-implies-size-change-031.html
[add] https://crrev.com/ca421e607aecefc211d829c996e21106190a1ff5/third_party/WebKit/LayoutTests/external/wpt/css/css-grid-1/alignment/grid-alignment-implies-size-change-032.html
[add] https://crrev.com/ca421e607aecefc211d829c996e21106190a1ff5/third_party/WebKit/LayoutTests/external/wpt/css/css-grid-1/alignment/grid-alignment-implies-size-change-033.html
[add] https://crrev.com/ca421e607aecefc211d829c996e21106190a1ff5/third_party/WebKit/LayoutTests/external/wpt/css/css-grid-1/alignment/grid-alignment-implies-size-change-034.html
[add] https://crrev.com/ca421e607aecefc211d829c996e21106190a1ff5/third_party/WebKit/LayoutTests/external/wpt/css/css-grid-1/alignment/grid-alignment-implies-size-change-035.html
[add] https://crrev.com/ca421e607aecefc211d829c996e21106190a1ff5/third_party/WebKit/LayoutTests/external/wpt/css/css-grid-1/alignment/grid-alignment-implies-size-change-036.html
[add] https://crrev.com/ca421e607aecefc211d829c996e21106190a1ff5/third_party/WebKit/LayoutTests/external/wpt/css/css-grid-1/support/style-change.js
[modify] https://crrev.com/ca421e607aecefc211d829c996e21106190a1ff5/third_party/WebKit/Source/core/layout/LayoutGrid.cpp
[modify] https://crrev.com/ca421e607aecefc211d829c996e21106190a1ff5/third_party/WebKit/Source/core/layout/LayoutGrid.h

Comment 7 by jfernan...@igalia.com, Sep 2 2017 

This issue should be FIXED now.

Comment 8 by jfernan...@igalia.com, Sep 2 2017

Status: Fixed (was: Started)
►

Sign in to add a comment