793686 - [css3-multicol] Column balancing breaks page-break-inside:avoid child

Status: RESOLVED FIXED

(Core :: Layout: Columns, defect)

Description

[Mats Palmgren (inactive)]

Attachment: Testcase #1

We need to be smarter about page-break-inside:avoid during column
balancing reflows.  See testcase, the desired rendering is a 1x1 inch
blue box in a single column.  Opera gets this right.

Comment 2

[Daniel Holbert [:dholbert]]

Note: per bug 1739561, this is now causing breakage in Gmail's "Trip" view (for travel-related emails), which display extracted bits of information in a multicol 'card' with various break-inside:avoid "units of information" inside of those cards. We're splitting up those units when we shouldn't be, due to this bug (or this category-of-bug).

Comment 3

[Ting-Yu Lin [:TYLin] (UTC-8)]

Yes, column balancing can still breaks a break-inside:avoid child. It is because the helper function ShouldAvoidBreakInside() returns false when a frame is at top of page/column. A parent frame definitely cannot push a break-inside:avoid child at the top of page to the next page/column to avoid breaking it. We might need to propagate the information all the way up to column balancing algorithm, and teach the algorithm to handle the status so that it is aware that the currently chosen column bsize is too small to fit the unbreakable descendant.

Comment 4

[Ting-Yu Lin [:TYLin] (UTC-8)]

Attachment: Bug 793686 Part 1 - Don't set mIsTopOfPage on -moz-column-content during column-balancing.

According to nsContainerFrame::ShouldAvoidBreakInside, we allow a frame to
break from inside when it is at the top of page/column, i.e. when mIsTopOfPage
is true.

This patch allows the content at the top of the column to honor the
break-inside:avoid during column balancing, so it can now propagate the
break-before status to nsColumnSetFrame if it needs to report such status.

The reference files are modified to reflect the rendering as of this patch. Note
that browsers do not agree on the rendering of 1420528-1.html. Before this
patch, our rendering is the same as Chrome by placing "Spacer" in the first
column, but now we agree with webkit by placing it in the second column.

Comment 5

[Ting-Yu Lin [:TYLin] (UTC-8)]

Attachment: Bug 793686 Part 2 - Call SetNextInFlowNeedsReflow() when an absolute containing block is incomplete.

This fixed the following assertion in nsColumnSetFrame::ReflowChildren() after
applying part 1 for layout/forms/crashtests/1690166-1.html.

ASSERTION: We have to create a continuation, but the block doesn't want us to reflow it?

Depends on D154048

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/fcf30cda2dce
Part 1 - Don't set mIsTopOfPage on -moz-column-content during column-balancing. r=emilio
https://hg.mozilla.org/integration/autoland/rev/b7e8fd18039a
Part 2 - Call SetNextInFlowNeedsReflow() when an absolute containing block is incomplete. r=emilio

Comment 7

[Sandor Molnar[:smolnar]]

Backed out for causing reftest failures

Backout link: https://hg.mozilla.org/integration/autoland/rev/615367610bb5ff8411add68f8aa79628fd90c157

Push with failures

Failure log

Comment 8

[Ting-Yu Lin [:TYLin] (UTC-8)]

The reftest failure in comment 7 is due to bug 1784002. I added a workaround in 611574-2.html in my latest revision.

https://treeherder.mozilla.org/jobs?repo=try&revision=9a6cfb1f70e16797747a606f5f280137f4e7ca65

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/0bfdd40111eb
Part 1 - Don't set mIsTopOfPage on -moz-column-content during column-balancing. r=emilio
https://hg.mozilla.org/integration/autoland/rev/cad5fc248dfe
Part 2 - Call SetNextInFlowNeedsReflow() when an absolute containing block is incomplete. r=emilio

Comment 10

[Andreea Pavel [:apavel]]

https://hg.mozilla.org/mozilla-central/rev/0bfdd40111eb
https://hg.mozilla.org/mozilla-central/rev/cad5fc248dfe