New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.
Starred by 7 users

Issue metadata

Status: Fixed
Owner:
Closed: Apr 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Android , Windows , Chrome , Mac
Pri: 2
Type: Bug-Regression



Sign in to add a comment

[css-grid] Input size breaks in a stretched grid container inside a flexbox

Reported by a...@gmx.net, Apr 4 2017 Back to list

Issue description

UserAgent: Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/57.0.2987.133 Safari/537.36

Steps to reproduce the problem:
1. open the flex-grid-bug.html
2. click into the first input field
3. enter some text

What is the expected behavior?
The size of the input should stay the same.

What went wrong?
The width of the input is reduced to padding + border size. 

Did this work before? N/A 

Does this work in other browsers? Yes

Chrome version: 57.0.2987.133  Channel: stable
OS Version: 10.0
Flash Version:
 
before.png
1.6 KB View Download
after.png
1.7 KB View Download
flex-grid-bug.html
877 bytes View Download
Cc: tkent@chromium.org gov...@chromium.org r...@igalia.com e...@chromium.org
Components: -Blink>Layout Blink>Layout>Grid
Labels: -Type-Bug ReleaseBlock-Stable M-57 M-59 M-58 Type-Bug-Regression
Status: Available
Able to reproduce the issue on Windows 7,10, Mac and Linux with latest Chrome Stable i.e., 57.0.2987.133 and Beta(58.0.3029.41), Dev and Canary(59.0.3062.0).


Please find the bisect range below :
You are probably looking for a change made after 433852 (known good), but no lat
er than 433853 (first known bad).
CHANGELOG URL:
  https://chromium.googlesource.com/chromium/src/+log/c11607a50691c6da823467bb389d14f659433c77..79bd413143afe5ad68104a3c99b9c04f64fc25ac

Labels: OS-Linux OS-Mac
Labels: OS-Android OS-Chrome
Since this is Blink related issue I am tagging Android and ChromeOS as well.
Cc: keta...@chromium.org abdulsyed@chromium.org amineer@chromium.org
Cc: sshruthi@chromium.org
+sshruthi@ for help with impact analysis

Comment 6 by svil...@igalia.com, Apr 5 2017

I can reproduce this, attaching an slightly simplified version of the issue (position absolute was not needed for example).

Interestingly the problem dissapears whenever either:
1) width/height are not fixed on the top level div
2) display:flex is removed
3) display:grid is removed

BR
fgb.html
408 bytes View Download

Comment 7 by r...@igalia.com, Apr 5 2017

Summary: [css-grid] Input size breaks in a stretched grid container inside a flexbox (was: Input size breaks in absolute-flex-grid-container combination)

I've a even more reduced test case.
Basically "align-items: stretch" on the flex container, is causing that the grid container has a 500px width.
That width is not changed at all when you put some text in the input.

Then the strange thing is what happens with the grid column.
Even if it's "auto" or "100%" or "1fr", when you enter some text it gets reduced to 0px.
Really strange


bug-708159-reduced.html
241 bytes View Download

Comment 8 by svil...@igalia.com, Apr 5 2017

I found the issue, the problem is that we're recomputing the intrinsic sizes after the layout because flexbox forces us to do so. This is the backtrace

LayoutBlock::computePreferredLogicalWidths
LayoutBox::maxPreferredLogicalWidth
LayoutFlexibleBox::childIntrinsicLogicalWidth
LayoutFlexibleBox::crossAxisIntrinsicExtentForChild
LayoutFlexibleBox::layoutAndPlaceChildren
LayoutFlexibleBox::layoutFlexItemsLayoutFlexibleBox::layoutBlock
LayoutBlock::layout

Now we need to figure out why our preferred logical width is dirty and fix that if it's wrong.

It might also be the case that we're clearing the overrides when we should not.

Otherwise we have a bigger issue, because as we know, the intrinsic size computation needs to modify the overrides used to implement grid areas. If the intrinsic size computation can happen at any moment then we should think about how to deal with that.

Comment 9 by svil...@igalia.com, Apr 5 2017

After thinking about it the problem is pretty clear. The preferred with is dirty because it was not computed before as the grid container is not intrinsically sized.

So the intrinsic size computation done after the layout is clearing the override sizes. That is the actual cause of the bug. I'll try to come up with a good fix for this.
Labels: -ReleaseBlock-Stable
Per dglazkov@ offline, css grid layouts are new and not used extensively; we don't need to consider this a release blocker.  That said if this is affecting non-grid layouts please let us know.
Cc: -amineer@chromium.org

Comment 12 by r...@igalia.com, Apr 6 2017

Owner: svil...@igalia.com
Status: Assigned
Cc: -sshruthi@chromium.org
un-cc-ing based on amineer's comment in c#10. We talked offline about this. If help is needed from me, please feel free to add me back.
Project Member

Comment 15 by bugdroid1@chromium.org, Apr 24 2017

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

commit 41d53a7316548ae2147de1fbb0cb9d1fda4ed3fa
Author: svillar <svillar@igalia.com>
Date: Mon Apr 24 17:21:00 2017

[css-grid] Preemptively store intrinsic sizes during layout

Computing intrinsic sizes after the layout process (as it happens in Mac
with the calls to content::RenderViewImpl::didUpdateLayout()) or when using
the grid as a flex item in some cases, is very problematic, because the
intrinsic size computation did change the override sizes of the items.

One potential solution would be to mark the grid as needs layout after the
preferred widths computation but that is forbidden by the engine (also it
would require an additional layout). There is a better way to do this based
on the fact that the preferred widths computation is actually part of the
track sizing algorithm.

We could store the intrinsic sizes of the grid returned by the track sizing
algorithm after computing the column sizes in the event of the intrinsic
sizes being dirty. Note that this does not affect the other cases where
preferred widths are required as we already execute
computeIntrinsicLogicalWidths() as part of updateLogicalWidths() if
required.

BUG= 708159 

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

[add] https://crrev.com/41d53a7316548ae2147de1fbb0cb9d1fda4ed3fa/third_party/WebKit/LayoutTests/fast/css-grid-layout/preferred-width-computed-after-layout.html
[modify] https://crrev.com/41d53a7316548ae2147de1fbb0cb9d1fda4ed3fa/third_party/WebKit/Source/core/layout/GridTrackSizingAlgorithm.cpp
[modify] https://crrev.com/41d53a7316548ae2147de1fbb0cb9d1fda4ed3fa/third_party/WebKit/Source/core/layout/GridTrackSizingAlgorithm.h
[modify] https://crrev.com/41d53a7316548ae2147de1fbb0cb9d1fda4ed3fa/third_party/WebKit/Source/core/layout/LayoutGrid.cpp

Comment 16 by svil...@igalia.com, Apr 25 2017

Labels: Merge-Request-58 Merge-Request-57 Merge-Request-59
Status: Fixed
Project Member

Comment 17 by sheriffbot@chromium.org, Apr 25 2017

Labels: -Merge-Request-59 Hotlist-Merge-Approved Merge-Approved-59
Your change meets the bar and is auto-approved for M59. Please go ahead and merge the CL to branch 3071 manually. Please contact milestone owner if you have questions.
Owners: amineer@(Android), cmasso@(iOS), gkihumba@(ChromeOS), Abdul Syed@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Project Member

Comment 18 by sheriffbot@chromium.org, Apr 25 2017

Labels: -Merge-Request-58 Merge-Review-58 Hotlist-Merge-Review
This bug requires manual review: Request affecting a post-stable build
Please contact the milestone owner if you have questions.
Owners: amineer@(Android), cmasso@(iOS), bhthompson@(ChromeOS), govind@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

Comment 19 by svil...@igalia.com, Apr 25 2017

Labels: -Merge-Request-57
Project Member

Comment 20 by bugdroid1@chromium.org, Apr 25 2017

Labels: -merge-approved-59 merge-merged-3071
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/f83ac4838f56cb809ddf3bfef76336979a2d4b09

commit f83ac4838f56cb809ddf3bfef76336979a2d4b09
Author: Sergio Villar Senin <svillar@igalia.com>
Date: Tue Apr 25 08:39:56 2017

[css-grid] Preemptively store intrinsic sizes during layout

Computing intrinsic sizes after the layout process (as it happens in Mac
with the calls to content::RenderViewImpl::didUpdateLayout()) or when using
the grid as a flex item in some cases, is very problematic, because the
intrinsic size computation did change the override sizes of the items.

One potential solution would be to mark the grid as needs layout after the
preferred widths computation but that is forbidden by the engine (also it
would require an additional layout). There is a better way to do this based
on the fact that the preferred widths computation is actually part of the
track sizing algorithm.

We could store the intrinsic sizes of the grid returned by the track sizing
algorithm after computing the column sizes in the event of the intrinsic
sizes being dirty. Note that this does not affect the other cases where
preferred widths are required as we already execute
computeIntrinsicLogicalWidths() as part of updateLogicalWidths() if
required.

BUG= 708159 

Review-Url: https://codereview.chromium.org/2834473003
Cr-Commit-Position: refs/heads/master@{#466668}
(cherry picked from commit 41d53a7316548ae2147de1fbb0cb9d1fda4ed3fa)

Review-Url: https://codereview.chromium.org/2839893002 .
Cr-Commit-Position: refs/branch-heads/3071@{#196}
Cr-Branched-From: a106f0abbf69dad349d4aaf4bcc4f5d376dd2377-refs/heads/master@{#464641}

[add] https://crrev.com/f83ac4838f56cb809ddf3bfef76336979a2d4b09/third_party/WebKit/LayoutTests/fast/css-grid-layout/preferred-width-computed-after-layout.html
[modify] https://crrev.com/f83ac4838f56cb809ddf3bfef76336979a2d4b09/third_party/WebKit/Source/core/layout/GridTrackSizingAlgorithm.cpp
[modify] https://crrev.com/f83ac4838f56cb809ddf3bfef76336979a2d4b09/third_party/WebKit/Source/core/layout/GridTrackSizingAlgorithm.h
[modify] https://crrev.com/f83ac4838f56cb809ddf3bfef76336979a2d4b09/third_party/WebKit/Source/core/layout/LayoutGrid.cpp

Comment 21 Deleted

Comment 22 by svil...@igalia.com, Apr 26 2017

Ups, I deleted comment 21 by mistake trying to reply to it, I'm sorry.

Anyway I'm fine with the rejection, it's already merged in M59 which is fine.
Cc: ranjitkan@chromium.org
Labels: TE-Verified-M59 TE-Verified-59.0.3071.29
Rechecked this issue on chrome version 59.0.2071.29 on Windows 10, MAC 10.12.4, Ubuntu 14.04. fix is working as intended. Clicking the input field retains the size of the input text field. Adding TE-verified labels. Attached screen shot for the same.

Thanks.!


Comment 24 by r...@igalia.com, May 10 2017

Cc: kavvaru@chromium.org svil...@igalia.com
 Issue 720064  has been merged into this issue.

Comment 25 by r...@igalia.com, May 31 2017

Cc: jfernan...@igalia.com
 Issue 727076  has been merged into this issue.
Labels: -Hotlist-Merge-Review
This bug is not just happing for display:flex; it is also happening on display: grid. Here's a video of it happening in Chromium Version 60.0.3112.113:
https://youtu.be/8-OmQUv5S9c

I'm experiencing this on "text", "email" and "tel" inputs that are laid via a CSS grid. The problem intensified as I added programming that changed focus automatically after the user completed certain fields.

Another example that intensified the behavior was simply trying to auto-focus on the first input field in the form. It seems like my focus() command occurred before the form was fully rendered or something. After this, when I'd manually click the first input in the form, it would collapse to a width that only included border and padding. That video shows this. Again:
https://youtu.be/8-OmQUv5S9c

Should I created a new bug, or is this the same exact issue even though it is for grid instead of flex?  

Comment 28 Deleted

Comment 29 Deleted

#27 - is  issue 727076  the same as you noted in your comment? If so, it is open and assigned, so it will be handled. If not, please, file a new issue (if you have not found an existing open one). You can comment here with the issue number.
#27 - also, you can check Chrome 61 (released a few days ago) and make sure your issue still reproduces. If not, no need to bother filing an issue. :)
Unfortunately, I have the same issue with inputs in grids that #27 has, but in Chrome 62 :/
 Bug #727076  was complex and took a while but it's fixed since October.
The first version that included the fix is 64.0.3250.0,
so the previous versions like 62 are still affected.

Sign in to add a comment