1674774 - Pagination/printing is quite broken for display:flex content in vertical-rl writing mode

Status: VERIFIED FIXED

(Core :: Layout: Flexbox, defect)

Description

[Jonathan Kew [:jfkthame]]

STR:

• Download the testcase from bug 1673499 and load the "minimal.html" file from the archive.
• Press Cmd-P to bring up the Print UI

Expected:
Some reasonable preview of how the document will print.

Actual:
Results vary depending on page size and print settings, but are not good. For example:

• With A4 paper size and "Fit to Page Width" selected, the entire document appears in tiny print with very long lines on a single page.
• With 100% scaling selected instead, the preview indicates the document is 3 pages, but none of the content is visible on any of the pages.

Note that if the writing-mode on the root element is changed from vertical-rl to vertical-lr, the print preview looks quite reasonable; it only seems to be vertical-rl mode where the pagination/layout breaks down.

Comment 1

[Jonathan Kew [:jfkthame]]

According to mozregression, it looks like this was affected by bug 1622935; prior to that, with scaling set to 100% the first page of the vertical-rl document would appear fine. (The remainder of the content would be dropped, as it wasn't getting fragmented successfully.)

After the landing of bug 1622935, we do fragment things, so we get 3 pages -- but the content has all disappeared out of view (perhaps misplaced to the left of the visible area?), which is hardly an improved experience!

Comment 2

[Jonathan Kew [:jfkthame]]

Attachment: reduced example

Here's a reduced example that exhibits broken flexbox-fragmentation behavior when you try to print-preview it (with scaling set to 100%).

(Note that if the root writing-mode is changed to vertical-lr, it fragments OK.)

Comment 3

[Ting-Yu Lin [:TYLin] (PDT, UTC-7)]

NI myself to take a look.

Comment 4

[BugBot [:suhaib / :marco/ :calixte]]

The component has been changed since the backlog priority was decided, so we're resetting it.
For more information, please visit auto_nag documentation.

Comment 5

[Ting-Yu Lin [:TYLin] (PDT, UTC-7)]

Attachment: Bug 1674774 Part 1 - Move two variables' definition closer to their usage.

Comment 6

[Ting-Yu Lin [:TYLin] (PDT, UTC-7)]

Attachment: Bug 1674774 Part 2 - Set block-end border and padding to zero in an edge case.

This patch fixed an edge case where the flex container's content
block-size can fit into the available block-size, but it becomes unfit
after adding block-end border and padding. In this case,
mayNeedNextInFlow should be true, and we should set block-end border
and padding to zero.

This doesn't change the behavior for now, but is needed by the next
part to determine the tentative border-box size.

Comment 7

[Ting-Yu Lin [:TYLin] (PDT, UTC-7)]

Attachment: Bug 1674774 Part 2 - Fix the border-box size which is used to position flex container's children.

If the flex container frame's tentative border-box size is different
from its final size, and it's in vertical-rl writing mode, we need to
adjust children's position. This is implemented in Part 4.

Comment 8

[Ting-Yu Lin [:TYLin] (PDT, UTC-7)]

Attachment: Bug 1674774 Part 3 - Move flex item's position when flex container has a vertical-rl writing-mode.

The flex container fragment's tentative block-size can be different from
its final size if there is any unbreakable child that has a block-size
larger than the available block-size. The two passed reftests are such
examples.

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/d4fa50b48efd
Part 1 - Move two variables' definition closer to their usage. r=mats
https://hg.mozilla.org/integration/autoland/rev/52c3e2fefc03
Part 2 - Fix the border-box size which is used to position flex container's children. r=mats
https://hg.mozilla.org/integration/autoland/rev/edf35fea5913
Part 3 - Move flex item's position when flex container has a vertical-rl writing-mode. r=mats

Comment 10

[Dorel Luca [:dluca]]

Backed out 3 changesets (bug 1674774) for Reftest failures in layout/reftests/flexbox/pagination/flexbox-single-column-vertical-rl-6.html. CLOSED TREE

Log:
https://treeherder.mozilla.org/logviewer?job_id=323062624&repo=autoland&lineNumber=28706

Push with failures:
https://treeherder.mozilla.org/jobs?repo=autoland&group_state=expanded&selectedTaskRun=FxjITxnoQi6eTAEqpFDUaQ.0&revision=edf35fea5913c869fd35f1e523967478edfcaa18

Backout:
https://hg.mozilla.org/integration/autoland/rev/d9b0105477475ee9d411fdd67d37b1774aedb3da

Comment 11

[Ting-Yu Lin [:TYLin] (PDT, UTC-7)]

Hmm, some reftests failed on Android because scrollbars show up.

Comment 12

[Ting-Yu Lin [:TYLin] (PDT, UTC-7)]

Filed bug 1679931 to fix the root cause of the extra scrollbar because of the wrong overflow area on block frame in vertical-rl writing-mode. I added annotations to failed reftests to workaround them.

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/7f28e2783cf1
Part 1 - Move two variables' definition closer to their usage. r=mats
https://hg.mozilla.org/integration/autoland/rev/7448c4ef16ca
Part 2 - Fix the border-box size which is used to position flex container's children. r=mats
https://hg.mozilla.org/integration/autoland/rev/cdb6f451342a
Part 3 - Move flex item's position when flex container has a vertical-rl writing-mode. r=mats

Comment 14

[Andreea Pavel [:apavel]]

https://hg.mozilla.org/mozilla-central/rev/7f28e2783cf1
https://hg.mozilla.org/mozilla-central/rev/7448c4ef16ca
https://hg.mozilla.org/mozilla-central/rev/cdb6f451342a

Comment 15

[Bogdan Maris, Desktop QA]

Reproduced the initial issue using an old Nightly from 2020-11-21, verified that this is fixed using latest Firefox Beta 85.0b4 across platforms (Windows 10, macOS 11 and Ubuntu 18.04).