Closed Bug 1602430 Opened 1 year ago Closed 1 year ago

min/max-height is applied incorrectly in fragmented flow

Categories

(Core :: Layout: Block and Inline, defect, P2)

defect

Tracking

()

RESOLVED FIXED
mozilla73
Tracking Status
firefox73 --- fixed

People

(Reporter: mats, Assigned: mats)

References

Details

(Keywords: testcase)

Attachments

(5 files)

Attached file Testcase

STR

  1. load the attached testcase
  2. Print Preview (A4 size)

EXPECTED RESULT
The word "AFTER" should be in the upper half of the second page, overlapped by the inner element with the borders.

ACTUAL RESULT
The word "AFTER" is near the end of the second page, after the inner element with the borders.

This is a pretty straight-forward fix... if it weren't for this rule:
https://searchfox.org/mozilla-central/rev/d24696b5abaf9fb75f7985952eab50d5f4ed52ac/layout/style/res/ua.css#264
which is pretty broken. If you have a column-set with say height:100px and three columns, then the max-block-size needs to be 300px not 100px (the max-block-size applies to the sum of the fragments, not each fragment individually).
What is this declaration needed for exactly?

Flags: needinfo?(aethanyc)

(In reply to Mats Palmgren (:mats) from comment #1)

This is a pretty straight-forward fix... if it weren't for this rule:
https://searchfox.org/mozilla-central/rev/d24696b5abaf9fb75f7985952eab50d5f4ed52ac/layout/style/res/ua.css#264
which is pretty broken.
If you have a column-set with say height:100px and three columns, then the max-block-size needs to be 300px not 100px (the max-block-size applies to the sum of the fragments, not each fragment individually).

You're talking about our implementation details that if we really want to set max-block-size to the column-set (or column-content?), it should be a 300px, right? Because I cannot find a spec text saying that the max-block-size specified on the multicol container should be a multiply of its block-size and column-count.

What is this declaration needed for exactly?

If I recall this correctly, this is for the legacy column container (with column-span disabled) which doesn't fully respect the column container's height when the container is fragmented. Nowadays, nsColumnSetFrame is an inner frame under ColumnSetWrapperFrame, it should fully respect the whatever available block-size passing down from ColumnSetWrapperFrame, and use it to impose the available block-size of column-content. The multicol's child contents shouldn't choose a block-size bigger than their available block-size.

Out of curiosity, I remove the max-block-size: 100%, and it breaks only three abspos dynamiclly height changing tests, which I guess is due to some optimization. I'd say we should remove the max-block-size rule.
https://treeherder.mozilla.org/#/jobs?repo=try&selectedJob=280433686&revision=7f9269054bc203737ded74f38f6bc69eb820c0a2

Flags: needinfo?(aethanyc)

Right, min/max-block-size applies to the CSS box, which means it applies to the sum of the heights of the fragments associated with that box (same as for block-size). In our column set implementation the columns are fragments, so it's no different from any other box. So, by specifying max-block-size: 100% only the first column has actual in-flow block-size and the remaining columns becomes overflow-containers, or should have anyway. The only reason that doesn't happen is that we don't actually apply min/max-block-size at all in fragmented contexts currently which I intend to fix in this bug.

I filed bug 1603088 to remove that declaration.

A couple of tests to illustrate just how broken our layout currently is. See Chrome for correct results.

Pushed by mpalmgren@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/b5c8349e4e35
Apply min/max-height correctly for fragmented boxes with no height specified.  r=TYLin
Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/20824 for changes under testing/web-platform/tests
Upstream web-platform-tests status checks passed, PR will merge once commit reaches central.
Status: NEW → RESOLVED
Closed: 1 year ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla73
Upstream PR merged by moz-wptsync-bot
Regressions: 1607045
No longer regressions: 1607045
QA Whiteboard: [frag_v73]
You need to log in before you can comment on or make changes to this bug.