To reduce the scope of bug 939897, we can support shifting or splitting flex items for single-line column and single-line row flex container.

Per 10.1. Sample Flex Fragmentation Algorithm in the spec, both single-line row and single-line column flex container needs to run the flex algorithm without regards to pagination, which can be easier to implement.

We need to do extra work when children is incomplete and that is also
what nsContainerFrame::PushIncompleteChildren() returns (we will call it
in Part 3).

Also, add relevant bits to be able to use them in the flex container.

Depends on D73165

Depends on D73166

Depends on D73167

Depends on D73168

Depends on D73169

Note: my current patches haven't considered advanced fragmentation properties like break-before, break-after, break-inside, nor dynamically inserting/appending/removing flex items. But they should be good enough to fix printing data loss.

(In reply to Ting-Yu Lin [:TYLin] (UTC-8) from comment #7)

Note: my current patches haven't considered advanced fragmentation properties like break-before, break-after, break-inside, nor dynamically inserting/appending/removing flex items. But they should be good enough to fix printing data loss.

Thanks for that clarification.

Also: I think it's implicit in the [frag_v78] whiteboard status, but just to be extra-sure: we should wait to land this until Monday 5/4, after the merge (after the soft freeze is lifted & nightly version is bumped to version 78). This would definitely benefit from a full Nightly cycle for testing.

(Also, we should go through the list of dupes on bug 939897 at some point and see how many of them are fixed by this patch-stack, while keeping an eye out for e.g. catastrophic crashes or similar, to be sure we're not making things worse somehow. :))

(In reply to Daniel Holbert [:dholbert] from comment #8)

Also: I think it's implicit in the [frag_v78] whiteboard status, but just to be extra-sure: we should wait to land this until Monday 5/4, after the merge (after the soft freeze is lifted & nightly version is bumped to version 78). This would definitely benefit from a full Nightly cycle for testing.

Yes, we should land this at the beginning of the Firefox 78 cycle. (We shouldn't land a major behavior change during the soft freeze anyway.)

(Also, we should go through the list of dupes on bug 939897 at some point and see how many of them are fixed by this patch-stack, while keeping an eye out for e.g. catastrophic crashes or similar, to be sure we're not making things worse somehow. :))

I'm planning to do what you've suggested! If some of the dupes on bug 937897 can be fixed by this bug, I'll make them dup over this bug instead. It will give us a better sense of the percentage of the real webpage that can be fixed, and perhaps give the reporters a heads-up that we do have some progress on it.

So for me, this testcase is still broken with this patch-stack applied.

In the top part of this testcase (the part with a fragmented flex container), the 2nd flex item with the "def" text ends up with its text overlapping the multicol's red bottom-border (i.e. we put that item somewhere without enough available space). This results in truncation if this is printing instead of multicol.

I think this might be a signal that we're reporting too large of an availableBSize to this item, perhaps? (just a guess) So it thinks it can fit at least that first line of text, when really it cannot.

(We do successfully push the third flex item ("ghi") correctly into the second column, in this patch. It's the second flex item that's problematic here.)

The broken testcase in comment 11 is somewhat expected for my current simple implementation because it simply lays the flex item out at where the flex algorithm tells it to and give the flex item whatever remaining available block-size, and punt the responsibility of breaking the item to the flex item itself.

(I'm aware that the test uses a more complex column-balancing scenario, so for the discussion below, I'll assume we use column-fill:auto on the multicol.)

Ideally, we could probably either do:

1. As suggested by step 3 of the sample single-line column flex container algo, we can push the flex item immediately to the next page/column if we know the remaining available space is less than 50% of the item's height. But it can also look bad even if the remaining available space is more than 50% of the item's height, because the item might be unbreakable at all (like one line of text). We can still lose the content in printing.

2. Ignore the 50% rule in the spec, and reflow the item anyway. If the item's desired height exceeds the remaining available height, push it to the next page/column.

If we implement pushing the entire flex item to the next page/column (maybe just for column-oriented flex container?), we'll need to reconsider some details of computing the flex container's height and the position offset of the pushed item.

In all, do you feel this should block this bug? Or maybe we can fix it in a follow-up?

(In reply to Ting-Yu Lin [:TYLin] (UTC-7) from comment #13)

2. Ignore the 50% rule in the spec, and reflow the item anyway. If the item's desired height exceeds the remaining available height, push it to the next page/column.

I think we should do this ^^. The 50% rule seems bogus & I don't think it's something we should follow. As you noted -- even if 51% of the fragment would fit in the available space, that's still means 49% dataloss for that fragment, which is still bad.

But for the purposes of this bug, your current approach (putting the item wherever it falls and deferring to it to handle fragmentation) seems like an incremental improvement & indeed probably doesn't need to be blocked on this, I think.

https://hg.mozilla.org/mozilla-central/rev/78d4dfbda06a
https://hg.mozilla.org/mozilla-central/rev/818c87e00d37
https://hg.mozilla.org/mozilla-central/rev/5c6cc9d819b9
https://hg.mozilla.org/mozilla-central/rev/6a21028d21a8
https://hg.mozilla.org/mozilla-central/rev/988074d5ce54
https://hg.mozilla.org/mozilla-central/rev/1035de8a28ce
https://hg.mozilla.org/mozilla-central/rev/76aa4bf4eabd

Change the title to reflect the fact that my patches can also fix print data loss of some real sites using multi-line row-oriented flex containers.

(In reply to Ting-Yu Lin [:TYLin] (UTC-7) from comment #18)

Change the title to reflect the fact that my patches can also fix print data loss of some real sites using multi-line row-oriented flex containers.

That's good news!

It might be worth checking in some automated testcases to cover those (in a test-only followup bug), to be sure we don't regress them in the time between now & when we "really" fix multi-line flex container fragmentation. What do you think?

Re comment 26:

Sure, I can add some reftests for multi-line row-oriented flex containers in bug 1637788 as a first step to figure out what's left to do. (And maybe some multi-line column-oriented ones if I see some real sites using it need to print.)

\o/ Amazing! A lots of duplicated bugs!