New issue
Advanced search Search tips

Issue 699683 link

Starred by 3 users

Issue metadata

Status: Fixed
Owner: ----
Closed: Mar 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 1
Type: Bug

Blocked on:
issue 699476



Sign in to add a comment

Layout Regression on top sites when display: flex is used

Project Member Reported by elawrence@chromium.org, Mar 8 2017

Issue description

Chrome Version: Canary 59.0.3035.0 and last few builds
Bisect returns https://chromium.googlesource.com/chromium/src/+log/9f04f39d54f599a346c59670f91923f4af05e416..f5a29c6561e3820a5e71e03c85eb7597b797bfd7

OS: Windows 10

What steps will reproduce the problem?
(1) Open https://developer.apple.com/reference/uikit/nsunderlinestyle
OBSERVE: Bottom of page scrolled off to the far right.
(2) Open Twitter DM conversation UI.
OBSERVE: Text Edit box scrolled out of view.

Of the CLs in range, https://chromium.googlesource.com/chromium/src/+/a90dba1f2dd094e51229dd2e5e059c6b53cee93a seems most suspect?

 
BadLayout.png
49.7 KB View Download
Cc: jfernan...@igalia.com
Labels: has-Bisect
jfernandez@ can you have a look? Apologies if I'm totally off.
After running "git revert a90dba1f2dd094e51229dd2e5e059c6b53cee93a" on my Canary build, both pages layout correctly.
Cc: e...@chromium.org
 Issue 699568  has been merged into this issue.
Summary: Layout Regression on top sites when display: flex is used (was: Layout Regression on top sites)
Related bug explains more clearly what is going on here ("Elements with display: flex and ::before display: row render content outside content box") and it includes a reduced test case.

Example URL:
http://codepen.io/tcschiller/pen/Qpdbjo

Steps to reproduce the problem:
The above codepen shows the issue. If flex on the element itself or table on the ::before element is removed, content renders correctly.
I haven't look at the issue in detail, but it may be related to my change, indeed.  

In fact, I'm investigating  issue #699116  as well, probably caused by the mentioned patch. 

Perhaps it'd be a good idea to revert it, so I can investigate both issues more carefully. 
https://crrev.com/2528253003 has been reverted. Could anybody verify the regression is solved now ?
After syncing to the latest (f78878b8ba3) and rebuilding Chromium, this is now fixed (verified on Twitter, Apple docs, and the codepen repro)
It might be a good idea to land a layout test for this, since it's pretty amazing that we didn't catch this in the CQ for the original change.
Labels: Merge-Request-59
Status: Started (was: Untriaged)
In any case, I think we should merge https://crrev.com/2740063003 in M59 as soon as possible.
Blockedon: 699476
Labels: -Merge-Request-59
Sorry about the confusion, no need to merge.
Status: Fixed (was: Started)
I'll mark this issue as FIXED then.
Thanks for verifying it.
I'll try to define a layout test for this issue as part of the proper fix of issue 667785, which patch has been the responsible of several regressions. I'll try also t define layout test cases based on them. 

Sign in to add a comment