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

Issue 674640 link

Starred by 19 users

Issue metadata

Status: Fixed
Owner:
NOT IN USE
Closed: Jan 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Android , Windows , Chrome , Mac
Pri: 1
Type: Bug-Regression



Sign in to add a comment

CSS3 Multi-column layout cropped whole columns with overflow-x: auto

Reported by martin.r...@xtheon.com, Dec 15 2016

Issue description

UserAgent: Mozilla/5.0 (Windows NT 6.3; WOW64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/55.0.2883.87 Safari/537.36

Steps to reproduce the problem:
1. Use Chrome 55
2. Create a multi-column layout
3. Use CSS overflow-x:auto

OR

1. Use Chrome 55
2. https://jsfiddle.net/odbjb3pv/2/

What is the expected behavior?
All columns should rendered properly.

What went wrong?
Only the first columns are rendered properly. The rest are not visible. DOM is OK.

Did this work before? Yes 54

Does this work in other browsers? Yes

Chrome version: 55.0.2883.87  Channel: stable
OS Version: 6.3
Flash Version: Shockwave Flash 24.0 r0

Test Code: https://jsfiddle.net/odbjb3pv/2/
 
Components: -Blink>CSS Blink>Layout>MultiCol Blink>Scroll
Status: Untriaged (was: Unconfirmed)
Components: -Blink>Scroll

Comment 3 by e...@chromium.org, Dec 15 2016

Labels: -Pri-2 Pri-1
Owner: msten...@opera.com
Status: Assigned (was: Untriaged)
Confirmed, worked in M54 but no in M55.

Comment 4 by msten...@opera.com, Dec 16 2016

I bisected with a simplified test case, and found that https://codereview.chromium.org/2522043002 has something to do with this. Reverting it doesn't fix the original jsfiddle test, though.

Comment 5 by ardrm...@gmail.com, Dec 17 2016

The same problem occurs with CSS overflow-x:visible; worked in previous versions

Comment 7 by msten...@opera.com, Dec 19 2016

Cc: msten...@opera.com
 Issue 674477  has been merged into this issue.

Comment 8 Deleted

Comment 9 by m.k...@texthelp.com, Dec 20 2016

Same occurs with overflow:hidden

https://jsfiddle.net/3mccu38j/2/
 Issue 677031  has been merged into this issue.
Cc: chrishtr@chromium.org gov...@chromium.org pbomm...@chromium.org
 Issue 677528  has been merged into this issue.

Comment 12 by qro...@gmail.com, Jan 17 2017

Adding transform: translateZ(0); partially fixes it.

Comment 13 by ardrm...@gmail.com, Jan 19 2017

Added transform: translateZ(0) but columns remain truncated (columns displayed with no content) while all other main browser show the content as intended.
We're seeing this for 'overflow: hidden;'.

Basically later columns (it may start at column 4, column 3, ...), depending on the container's width, are not rendering (however the debugger seems to know elements should be that and has their correct positions computed basic on however over the elements in the Elements tab).

Any word on a fix? This is pretty critical for us as a feature was designed when Chrome 54 was out and this was working and now it's useless on Chrome.

- Thanks, Alan

Comment 15 by msten...@opera.com, Jan 20 2017

I've been working on a fix this week. I'm trying hard to come up with a patch that doesn't cause regressions. Right now there's one test that breaks, albeit breaking that test is less serious than having this bug. So I might end up doing just that.

As a work-around, while you're waiting for a fix, would it work to wrap the multicol container inside a DIV with overflow:hidden and let the multicol container itself have overflow:visible?

E.g. this:

<div id="scrollable" style="overflow:hidden;">
    <div style="columns:3;">
       ...
    </div>
</div>

Rather than this:

<div id="scrollable" style="overflow:hidden; columns:3;">
    ...
</div>
Thanks for the quick response and the workaround.

I'm going to have to see if we can make that workaround work in our real code... I was able to make that work for my test page.

Still looking forward to the 'real' fix though ;-)

- Thanks, Alan
Since I am using Dojo, I cannot implement the proposed workaround.

This is a critical bug for us.  

Any guess at time frame for a fix?

Thank you very much,
John

Comment 18 by msten...@opera.com, Jan 23 2017

The fix is in code review, so once the reviewer is happy, I'll land it. Then I guess we need to backport it to Chrome 56 and 55. By the end of the week should be doable.
Project Member

Comment 19 by bugdroid1@chromium.org, Jan 23 2017

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

commit ccde605b5967c796069c4208227f088b62bc7483
Author: mstensho <mstensho@opera.com>
Date: Mon Jan 23 20:52:19 2017

Don't cancel out scroll offset when calculating the clip rectangle for multicol.

We still want the clip rect to be relative to the multicol container, but we
cannot get there by using the location() of the flow thread's PaintLayer,
because then we'll then cancel out the scroll offset that's also baked into
offsetOfPaginationLayerFromRoot.

This CL will cause paint/invalidation/paged-with-overflowing-block-rl.html to
regress, but it turned out that it just passed by accident anyway. Having that
test broken is way less serious than barely being able to scroll at all in a
regular multicol container.

BUG= 674640 
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2

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

[modify] https://crrev.com/ccde605b5967c796069c4208227f088b62bc7483/third_party/WebKit/LayoutTests/FlagExpectations/enable-slimming-paint-v2
[modify] https://crrev.com/ccde605b5967c796069c4208227f088b62bc7483/third_party/WebKit/LayoutTests/TestExpectations
[add] https://crrev.com/ccde605b5967c796069c4208227f088b62bc7483/third_party/WebKit/LayoutTests/fast/multicol/scrollable-basic-expected.html
[add] https://crrev.com/ccde605b5967c796069c4208227f088b62bc7483/third_party/WebKit/LayoutTests/fast/multicol/scrollable-basic.html
[modify] https://crrev.com/ccde605b5967c796069c4208227f088b62bc7483/third_party/WebKit/Source/core/paint/PaintLayer.cpp

Comment 20 by msten...@opera.com, Jan 24 2017

Labels: Merge-Request-56 Merge-Request-55
Project Member

Comment 21 by sheriffbot@chromium.org, Jan 24 2017

Labels: -Merge-Request-56 Merge-Review-56 Hotlist-Merge-Review
This bug requires manual review: We are only 6 days from stable.
Please contact the milestone owner if you have questions.
Owners: amineer@(clank), cmasso@(bling), gkihumba@(cros), bustamante@(desktop)

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

Comment 22 by msten...@opera.com, Jan 24 2017

Cc: amineer@chromium.org
Cc: bustamante@chromium.org
Labels: -Merge-Request-55 Merge-Approved-57
We're not shipping anymore 55 releases, so removing that tag.  You missed the 57 branch, so pre-approving for merge there, please complete this merge.

For 56, the next release to desktop is going straight to stable, so we have no tolerance for risk.  Is this patch completely safe?  Is it absolutely required?  +bustamante as FYI.

Also, is this really Windows only?  This should affect all platforms, yes?
Please merge your change to M57 branch 2987 ASAP so we can pick it up for this week dev release. Thank you.
Labels: -Merge-Review-56 Merge-Rejected-56 OS-Android OS-Chrome OS-Linux OS-Mac
Confirmed this affects all platforms (aside from iOS).  Setting appropriate tags.

I spoke with eae@ offline on this; since the next release is shipping straight to stable, they feel this carries too much risk to merge at the 11th hour.  We'll have to wait for M57.
Full rationale for the non-merge (in addition to riskiness of fix): "[This issue] is a combination of two rarely used properties for a new feature with limited adoption."

FWIW, I apologize that this is broken currently - I hate leaving the web platform in a bad state for anyone.  If the fix was less risky I'd happily merge it, but we have to balance addressing bugs like this with possibly breaking things for our ~1B other users (e.g. if a regression is introduced with a late patch like this).  We'll work to investigate what we can do to prioritize fixing issues like this sooner.
Project Member

Comment 27 by bugdroid1@chromium.org, Jan 24 2017

Labels: -merge-approved-57 merge-merged-2987
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/12a56329208e800d464f2692db6edb2505c262b9

commit 12a56329208e800d464f2692db6edb2505c262b9
Author: Morten Stenshorne <mstensho@opera.com>
Date: Tue Jan 24 20:22:43 2017

Don't cancel out scroll offset when calculating the clip rectangle for multicol.

We still want the clip rect to be relative to the multicol container, but we
cannot get there by using the location() of the flow thread's PaintLayer,
because then we'll then cancel out the scroll offset that's also baked into
offsetOfPaginationLayerFromRoot.

This CL will cause paint/invalidation/paged-with-overflowing-block-rl.html to
regress, but it turned out that it just passed by accident anyway. Having that
test broken is way less serious than barely being able to scroll at all in a
regular multicol container.

BUG= 674640 
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2

Review-Url: https://codereview.chromium.org/2643123004
Cr-Commit-Position: refs/heads/master@{#445478}
(cherry picked from commit ccde605b5967c796069c4208227f088b62bc7483)

Review-Url: https://codereview.chromium.org/2651713006 .
Cr-Commit-Position: refs/branch-heads/2987@{#71}
Cr-Branched-From: ad51088c0e8776e8dcd963dbe752c4035ba6dab6-refs/heads/master@{#444943}

[modify] https://crrev.com/12a56329208e800d464f2692db6edb2505c262b9/third_party/WebKit/LayoutTests/FlagExpectations/enable-slimming-paint-v2
[modify] https://crrev.com/12a56329208e800d464f2692db6edb2505c262b9/third_party/WebKit/LayoutTests/TestExpectations
[add] https://crrev.com/12a56329208e800d464f2692db6edb2505c262b9/third_party/WebKit/LayoutTests/fast/multicol/scrollable-basic-expected.html
[add] https://crrev.com/12a56329208e800d464f2692db6edb2505c262b9/third_party/WebKit/LayoutTests/fast/multicol/scrollable-basic.html
[modify] https://crrev.com/12a56329208e800d464f2692db6edb2505c262b9/third_party/WebKit/Source/core/paint/PaintLayer.cpp

Comment 28 by msten...@opera.com, Jan 24 2017

Status: Fixed (was: Assigned)
Thanks for the prompt updates. We weren't able to make the workaround work in our production code so we're anxiously awaiting the fix. In the meantime we've had to disable this feature in Chrome. So V57 should have this in barring no issues in beta builds?

- Thanks, Alan

Comment 30 by msten...@opera.com, Jan 25 2017

Correct.

Sign in to add a comment