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

Issue 757947 link

Starred by 1 user

Issue metadata

Status: Available
Owner: ----
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux
Pri: 3
Type: Bug



Sign in to add a comment

Flow thread of inner multicol should be the one established by the outer multicol container

Project Member Reported by robho...@gmail.com, Aug 22 2017

Issue description

"The containing flow thread of the spanner in an inner multicol should ideally be the flow thread established by the outer multicol container, that's not how it's done in LocateFlowThreadContainingBlockOf()."

See LayoutTests/fast/multicol/span/thead-in-spanner-crash.html
 

Comment 1 by robho...@gmail.com, Aug 22 2017

Cc: msten...@opera.com robhogan@chromium.org
Components: Blink>Layout>MultiCol

Comment 2 by e...@chromium.org, Aug 22 2017

Owner: msten...@opera.com
Status: Assigned (was: Unconfirmed)
Project Member

Comment 3 by bugdroid1@chromium.org, Aug 25 2017

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

commit b71ac55bffca1c376d6ed4e540194436daccfbd1
Author: Morten Stenshorne <mstensho@opera.com>
Date: Fri Aug 25 16:35:23 2017

Locate the correct flow thread for spanners and out-of-flow objects.

LayoutFlowThread::LocateFlowThreadContainingBlockOf() used to just
give up if the nearest ancestor flow thread of some object wasn't the
containing flow thread.

The machinery that maintains the special-objects
(LayoutMultiColumnSet, LayoutMulticolumnSpannerPlaceholder) for
multicol still needs to ignore out-of-flow objects and spanners,
though, so I moved the check for this from
LocateFlowThreadContainingBlockOf() to
FlowThreadDescendantWasInserted() and
FlowThreadDescendantWillBeRemoved().

Checking on the paint side whether page logical height is known is
no longer necessary.

Bug: 757947
Cq-Include-Trybots: master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
Change-Id: Ief0492e9de52b01b08c3bc0318cb3c8abe67ccd3
Reviewed-on: https://chromium-review.googlesource.com/632057
Reviewed-by: Emil A Eklund <eae@chromium.org>
Commit-Queue: Morten Stenshorne <mstensho@opera.com>
Cr-Commit-Position: refs/heads/master@{#497437}
[modify] https://crrev.com/b71ac55bffca1c376d6ed4e540194436daccfbd1/third_party/WebKit/LayoutTests/FlagExpectations/enable-blink-features=LayoutNG
[modify] https://crrev.com/b71ac55bffca1c376d6ed4e540194436daccfbd1/third_party/WebKit/LayoutTests/FlagExpectations/enable-slimming-paint-v2
[add] https://crrev.com/b71ac55bffca1c376d6ed4e540194436daccfbd1/third_party/WebKit/LayoutTests/fast/multicol/out-of-flow/nested-with-abspos-inside-relpos-expected.html
[add] https://crrev.com/b71ac55bffca1c376d6ed4e540194436daccfbd1/third_party/WebKit/LayoutTests/fast/multicol/out-of-flow/nested-with-abspos-inside-relpos.html
[modify] https://crrev.com/b71ac55bffca1c376d6ed4e540194436daccfbd1/third_party/WebKit/Source/core/layout/LayoutFlowThread.cpp
[modify] https://crrev.com/b71ac55bffca1c376d6ed4e540194436daccfbd1/third_party/WebKit/Source/core/layout/LayoutMultiColumnFlowThread.cpp
[modify] https://crrev.com/b71ac55bffca1c376d6ed4e540194436daccfbd1/third_party/WebKit/Source/core/layout/LayoutMultiColumnFlowThread.h
[modify] https://crrev.com/b71ac55bffca1c376d6ed4e540194436daccfbd1/third_party/WebKit/Source/core/paint/TableSectionPainter.cpp

Comment 4 by msten...@opera.com, Aug 25 2017

Status: Fixed (was: Assigned)

Comment 5 by msten...@opera.com, Aug 28 2017

Status: Assigned (was: Fixed)
Reverting the fix, since it caused too much trouble. Probably fixable, but I don't have time for that now.
Project Member

Comment 6 by bugdroid1@chromium.org, Aug 28 2017

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

commit 47d61290f9f82f13f775bd43e57e6437e7638512
Author: Morten Stenshorne <mstensho@opera.com>
Date: Mon Aug 28 17:40:47 2017

Revert "Locate the correct flow thread for spanners and out-of-flow objects."

This reverts commit b71ac55bffca1c376d6ed4e540194436daccfbd1.

Reason for revert: Opened up a can of worms. Re-seal it.

Original change's description:
> Locate the correct flow thread for spanners and out-of-flow objects.
> 
> LayoutFlowThread::LocateFlowThreadContainingBlockOf() used to just
> give up if the nearest ancestor flow thread of some object wasn't the
> containing flow thread.
> 
> The machinery that maintains the special-objects
> (LayoutMultiColumnSet, LayoutMulticolumnSpannerPlaceholder) for
> multicol still needs to ignore out-of-flow objects and spanners,
> though, so I moved the check for this from
> LocateFlowThreadContainingBlockOf() to
> FlowThreadDescendantWasInserted() and
> FlowThreadDescendantWillBeRemoved().
> 
> Checking on the paint side whether page logical height is known is
> no longer necessary.
> 
> Bug: 757947
> Cq-Include-Trybots: master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
> Change-Id: Ief0492e9de52b01b08c3bc0318cb3c8abe67ccd3
> Reviewed-on: https://chromium-review.googlesource.com/632057
> Reviewed-by: Emil A Eklund <eae@chromium.org>
> Commit-Queue: Morten Stenshorne <mstensho@opera.com>
> Cr-Commit-Position: refs/heads/master@{#497437}

TBR=mstensho@opera.com,robhogan@gmail.com,eae@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug: 757947
Change-Id: Idf54d6cea2c03871c3a3443880f6b5b331cd7d30
Cq-Include-Trybots: master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
Reviewed-on: https://chromium-review.googlesource.com/637870
Reviewed-by: Morten Stenshorne <mstensho@opera.com>
Commit-Queue: Morten Stenshorne <mstensho@opera.com>
Cr-Commit-Position: refs/heads/master@{#497791}
[modify] https://crrev.com/47d61290f9f82f13f775bd43e57e6437e7638512/third_party/WebKit/LayoutTests/FlagExpectations/enable-blink-features=LayoutNG
[modify] https://crrev.com/47d61290f9f82f13f775bd43e57e6437e7638512/third_party/WebKit/LayoutTests/FlagExpectations/enable-slimming-paint-v2
[delete] https://crrev.com/b92fa52719c8826a96d85e19f0b204c76998f523/third_party/WebKit/LayoutTests/fast/multicol/out-of-flow/nested-with-abspos-inside-relpos-expected.html
[delete] https://crrev.com/b92fa52719c8826a96d85e19f0b204c76998f523/third_party/WebKit/LayoutTests/fast/multicol/out-of-flow/nested-with-abspos-inside-relpos.html
[modify] https://crrev.com/47d61290f9f82f13f775bd43e57e6437e7638512/third_party/WebKit/Source/core/layout/LayoutFlowThread.cpp
[modify] https://crrev.com/47d61290f9f82f13f775bd43e57e6437e7638512/third_party/WebKit/Source/core/layout/LayoutMultiColumnFlowThread.cpp
[modify] https://crrev.com/47d61290f9f82f13f775bd43e57e6437e7638512/third_party/WebKit/Source/core/layout/LayoutMultiColumnFlowThread.h
[modify] https://crrev.com/47d61290f9f82f13f775bd43e57e6437e7638512/third_party/WebKit/Source/core/paint/TableSectionPainter.cpp

Comment 7 by msten...@opera.com, Aug 28 2017

Reverted, because it caused  bug 759338 .
Project Member

Comment 8 by sheriffbot@chromium.org, Oct 20 2017

Labels: Hotlist-Recharge-BouncingOwner
Owner: ----
Status: Untriaged (was: Assigned)
The assigned owner "mstensho@opera.com" is not able to receive e-mails, please re-triage.

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

Comment 9 by e...@chromium.org, Oct 30 2017

Cc: -msten...@opera.com
Status: Available (was: Untriaged)
Owner: mstensho@chromium.org
Status: Assigned (was: Available)
Cc: mstensho@chromium.org
Owner: ----
Status: Available (was: Assigned)

Sign in to add a comment