Closed Bug 1588966 Opened 2 years ago Closed 9 months ago

The block-end border and padding are always subtracted from available content area if the column container is in paginated context

Categories

(Core :: Layout: Columns, defect, P3)

defect

Tracking

()

RESOLVED FIXED
87 Branch
Tracking Status
firefox87 --- fixed

People

(Reporter: TYLin, Assigned: TYLin)

Details

Attachments

(7 files, 1 obsolete file)

The bug happens because we always subtract block-end border and padding at
https://searchfox.org/mozilla-central/rev/171109434c6f2fe086af3b2322839b346a112a99/layout/generic/nsBlockFrame.cpp#3569-3573

Note: This bug also happens with column-span disabled because nsColumnSetFrame::GetAvailableContentBSize() always ignores the block-end skip-side, and subtracts block-end border and padding from the available-size.

As you can see in the attached test case, there is 20px space in every column content boxes in the inner column.

If the column containers (ColumnSetWrapper) is in a paginated context, we
incorrect subtract its block-end border and padding from the available
remaining block-size to its every fragments even if it doesn't have
"box-decoration-break: clone".

We already have multicol-breaking-004 covering
"box-decoration-break:clone" for a column container in a paginated
context.

Note: This bug also happens with column-span disabled because
nsColumnSetFrame::GetAvailableContentBSize() always ignores the
block-end skip side, and subtracts block-end border and padding from the
available-size.

Attached image large-border-bottom.png

This is a screenshot for the test case https://bugzilla.mozilla.org/attachment.cgi?id=9101441 by changing border-bottom to 100px on the inner column container (with my patch applied). It looks really worst on current Nightly, btw.

Mats, do you know what is the expected result in this case? We don't seem to create a next-in-flow when the content is complete, but the border and padding overflow the fragment.

Flags: needinfo?(mats)

For comparison with comment 2, this is Chrome result with border-bottom 100px on the inner column container.

Chrome's rendering in comment 3 is most definitely not spec-compliant. A border should never be split like that when there are other break opportunities.
https://drafts.csswg.org/css-break-3/#possible-breaks
https://drafts.csswg.org/css-break-3/#unforced-breaks

Flags: needinfo?(mats)

This case is probably hard to fix without implementing a more advanced handling of break opportunities in general. That is, we might want to investigate architectural changes first.

For comparison, here's the same test but with .inner being an ordinary block instead of a column-set. It has the exact same behavior as the column-set issue you're trying to address here.

Agree with Mats in comment 5. We might need a better vocabulary of the spec's Class A, B, C break points in our code to get a better spec-compliant frgmentation implementation.

FWIW, I came across bug 743402 comment 12, where roc and fantasai suggested we always include bottom border and padding when reflowing the content, which is our current behavior for years. And bug 743402 comment 13 is the suggestion to the code that leads to our current nsColumnSetFrame::GetAvailableContentBSize() implementaion.

Assignee: aethanyc → nobody
Status: ASSIGNED → NEW

bug 1677917 changed the available content area in a way similar to what I had envisioned, so I'll drop D49523. multicol-breaking-006.html is still worth adding as a wpt reftest because now we render it the same as in Chrome. I'll upload another patch to add it.

Attachment #9101742 - Attachment is obsolete: true

bug 1677917 makes Firefox render this test the same as Google Chrome.

Assignee: nobody → aethanyc
Status: NEW → ASSIGNED
Pushed by aethanyc@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/7d6747360f80
Add a wpt reftest of breaking a nested column container. r=emilio
Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/27419 for changes under testing/web-platform/tests
Status: ASSIGNED → RESOLVED
Closed: 9 months ago
Resolution: --- → FIXED
Target Milestone: --- → 87 Branch
Upstream PR merged by moz-wptsync-bot
You need to log in before you can comment on or make changes to this bug.