Improve page break related to inline(-block) elements #2497

Mellthas · 2021-07-27T16:03:02Z

Inline frames now properly report their height, the same as text frames. This resolves inline frames extending into the page margins instead of breaking onto a new page.
The positioning of inline frames has also been improved, allowing line breaks within them in more instances (instead of breaking the whole frame).
Allow line breaks and page breaks before inline-block elements, which were not allowed before.

Nested inline frames with margin, border, or padding still cause bugs at line breaks, because they are added to the text children on-the-fly. To properly fix these cases, it would probably be best to refactor the code in such a way that margin, border, and padding are properly handled on the inline frame itself, instead of being added to first resp. last child. It might be even worth considering always containing text frames in an anonymous inline box if needed; that could allow handling these properties consistently while simplifying the code. But the changes should improve the situation for now, nonetheless.

I have a few test cases in preparation that will demonstrate several broken scenarios that are fixed by this PR.

Mellthas · 2021-07-29T12:01:50Z

I have added one more commit that should improve auto-width calculation for inline-block and floating elements by sticking more closely to the spec.

Mellthas · 2021-08-01T16:00:33Z

Added another commit to trim trailing whitespace after line breaks in more cases, notably after inline and inline-block frames.

Mellthas · 2021-08-03T15:38:16Z

For a sample of the scenarios the commits here intend to fix, see the test cases included and listed in #2510.

bsweeney · 2021-09-11T23:51:22Z

Lots to review here, in the meantime when running through my standard test bed I noticed two issues. I haven't had a chance to look and see what's going on yet, so here's the errors.

First, input buttons render with no width:

<input type="submit" value="submit" />

Second, rendering http://eclecticgeek.com/dompdf/core_tests/encoding_utf-8_all.html throws the following exception:

[11-Sep-2021 19:33:25 America/New_York] 
[11-Sep-2021 19:33:25 America/New_York] TypeError: Return value of Dompdf\FrameReflower\Text::_collapse_white_space() must be of the type string, null returned in dompdf\src\FrameReflower\Text.php:86
Stack trace:
[0] dompdf\src\FrameReflower\Text.php(261): Dompdf\FrameReflower\Text->_collapse_white_space('D800  \xED\xA0\x80\xED\xA0\x81\xED\xA0\x82...')
[1] dompdf\src\FrameReflower\Text.php(396): Dompdf\FrameReflower\Text->_layout_line()
[2] dompdf\src\FrameDecorator\AbstractFrameDecorator.php(902): Dompdf\FrameReflower\Text->reflow(Object(Dompdf\FrameDecorator\Block))
[3] dompdf\src\FrameReflower\Block.php(872): Dompdf\FrameDecorator\AbstractFrameDecorator->reflow(Object(Dompdf\FrameDecorator\Block))
[4] dompdf\src\FrameDecorator\AbstractFrameDecorator.php(902): Dompdf\FrameReflower\Block->reflow(NULL)
[5] dompdf\src\FrameReflower\Page.php(141): Dompdf\FrameDecorator\AbstractFrameDecorator->reflow()
[6] dompdf\src\FrameDecorator\AbstractFrameDecorator.php(902): Dompdf\FrameReflower\Page->reflow(NULL)
[7] dompdf\src\Dompdf.php(837): Dompdf\FrameDecorator\AbstractFrameDecorator->reflow()
[8] test\run-tests.php(145): Dompdf\Dompdf->render()
[9] {main}

Mellthas · 2021-09-12T14:22:52Z

The second error is due preg_replace returning null in case of an error. In case of https://eclecticgeek.com/dompdf/core_tests/encoding_utf-8_all.html, it seems to be related to the surrogate code points starting at D800. Notice how there is no output for these when running through the debug helper at https://eclecticgeek.com. On my system (Ubuntu 20.04 with the default PHP 7.4.3), preg_replace runs without errors in these cases, the resulting output has the characters included as question marks (which I think is as expected). See the attached output file.

I have pushed an update to resolve the error by explicitly retaining the current behavior: Effectively returning the empty string in case of a null return value of preg_replace. Whether that behavior is fine, or if it would make more sense to preserve the original string in case of an error is I think a discussion unrelated to the current pull request.

Mellthas · 2021-09-12T14:25:31Z

By the way, there is a broken br tag down there in encoding_utf-8_all.html ;-).

Mellthas · 2021-09-12T14:36:42Z

Input buttons rendering without width is probably related to the change to use shrink-to-fit width resolution for float and inline-block elements. Generated content is only set right before reflowing the generated-content frame, so it is not yet set when calling get_min_max(_content)_width. This already affects the width calculations for table cells.

Mellthas · 2021-09-12T17:03:37Z

I don't see a straight-forward way for handling the generated content right now, so I am going to split up the PR.

For simplicity, return a `float` value consistently. The `get_break_margins` method was unused.

Also, add type declarations to `_collapse_white_space`.

Consistent argument names and more type declarations.

Always evaluated to `0` before, because their style's height is always `auto`. The new method is a copy of the one in `FrameDecorator/Text`. Woud be nice to refactor this in the future.

The original code would: * Not position inline frames properly, which lead to erroneous interaction with page breaks * Break inline frames to early, if the first word in the frame was not the longest one (unless in a fixed-position parent) * Not split inline frames properly, if they had text-node children after other children * Never move inline-block frames to a new line * Never allow page breaks before inline-block frames

Fixes backgrounds or borders bleeding into the page margins at page- break locations. This is achieved by fixing and using the existing `remove_frames_from_line` method.

See the linked spec sections: https://www.w3.org/TR/CSS21/visudet.html#float-width https://www.w3.org/TR/CSS21/visudet.html#inlineblock-width

Originally introduced in commit 542483a for calculating auto widths for float and inline-block frames.

Move the logic for trimming trailing whitespace after a line break to the `add_line` method of `BlockFrameDecorator`, so it is always applied when a new line box is added. This now also trims trailing whitespace if a text frame is used in block formatting context, but this may even be desirable. Also, restore trailing whitespace removed in this way after a split to address issues such as dompdf#2250.

The condition is wrong, as an `auto` width can resolve to more than 100% if the minimum width of the content is larger.

Per spec, `min-width` overrules `max-width`. https://www.w3.org/TR/CSS21/visudet.html#min-max-widths

Mellthas · 2021-09-12T18:10:33Z

The split is done: I would review #2545, #2546, #2547 first, then #2548. That finally leaves #2549, which might require more work to prevent the mentioned regressions related to generated content.

bsweeney · 2021-09-12T19:00:28Z

By the way, there is a broken br tag down there in encoding_utf-8_all.html ;-).

heh, yes thanks

bsweeney · 2021-09-12T19:03:51Z

The split is done: I would review #2545, #2546, #2547 first, then #2548. That finally leaves #2549, which might require more work to prevent the mentioned regressions related to generated content.

Are those replacements for this PR or in addition to it? Should I go ahead and finish reviewing this one before I move on to those?

Mellthas · 2021-09-12T19:29:15Z

Those PRs replace this one completely, so no need to continue reviewing here.

Mellthas mentioned this pull request Jul 27, 2021

Negative margins causing a line break with inline elements #2476

Closed

Mellthas force-pushed the page-break branch from 9dfe90f to 108fa82 Compare July 29, 2021 19:15

This was referenced Jul 29, 2021

Implement support for widows when determining the location of page breaks #1356

Open

position: relative; does not work on neither inline nor inline-block elements. #516

Closed

td with no dim spec breaks table rendering #2366

Closed

Mellthas force-pushed the page-break branch 3 times, most recently from 3a0340a to 48b7e88 Compare August 1, 2021 15:59

Mellthas force-pushed the page-break branch from 48b7e88 to 7864b21 Compare August 1, 2021 22:12

Mellthas mentioned this pull request Aug 3, 2021

Implement a basic framework for output tests #2510

Merged

Mellthas force-pushed the page-break branch from 7864b21 to f11ad82 Compare August 11, 2021 20:25

Mellthas mentioned this pull request Aug 11, 2021

Missing white spaces at the end of long lines #2250

Closed

Mellthas force-pushed the page-break branch from f11ad82 to ce7e7d6 Compare August 11, 2021 20:48

bsweeney linked an issue Aug 24, 2021 that may be closed by this pull request

td with no dim spec breaks table rendering #2366

Closed

bsweeney added this to the 1.0.3 milestone Aug 24, 2021

Mellthas force-pushed the page-break branch 2 times, most recently from c9eb295 to ce04cf4 Compare September 1, 2021 21:26

This was linked to issues Sep 9, 2021

position: relative; does not work on neither inline nor inline-block elements. #516

Closed

Missing white spaces at the end of long lines #2250

Closed

Mellthas force-pushed the page-break branch from ce04cf4 to 4298661 Compare September 12, 2021 13:59

Mellthas added 2 commits September 12, 2021 19:30

Clean up get_margin_height/width functions

b6b2f53

For simplicity, return a `float` value consistently. The `get_break_margins` method was unused.

TextReflower: Small cleanup for get_min_max_width

b3b19b3

Also, add type declarations to `_collapse_white_space`.

Clean up split method signatures

555a233

Consistent argument names and more type declarations.

Mellthas mentioned this pull request Sep 12, 2021

Small code cleanups #2547

Merged

Mellthas added 8 commits September 12, 2021 19:42

Report a correct margin height for inline elements

beb16be

Always evaluated to `0` before, because their style's height is always `auto`. The new method is a copy of the one in `FrameDecorator/Text`. Woud be nice to refactor this in the future.

Remove frames from line boxes after split

96647d6

Fixes backgrounds or borders bleeding into the page margins at page- break locations. This is achieved by fixing and using the existing `remove_frames_from_line` method.

Use shrink-to-fit width resolution for float and inline-block

6f06465

See the linked spec sections: https://www.w3.org/TR/CSS21/visudet.html#float-width https://www.w3.org/TR/CSS21/visudet.html#inlineblock-width

Remove now unused functions for auto-width calculations

ae2e300

Originally introduced in commit 542483a for calculating auto widths for float and inline-block frames.

BlockFrameReflower: Fix handling of 100% width

7e38142

The condition is wrong, as an `auto` width can resolve to more than 100% if the minimum width of the content is larger.

BlockFrameReflower: Remove swapping of min-/max-width

b1499a3

Per spec, `min-width` overrules `max-width`. https://www.w3.org/TR/CSS21/visudet.html#min-max-widths

Mellthas force-pushed the page-break branch from 4298661 to b1499a3 Compare September 12, 2021 17:43

This was referenced Sep 12, 2021

Improve page break related to inline(-block) elements #2548

Merged

Use shrink-to-fit width resolution for float and inline-block #2549

Merged

Mellthas closed this Sep 12, 2021

Mellthas deleted the page-break branch September 16, 2021 18:50

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Improve page break related to inline(-block) elements #2497

Improve page break related to inline(-block) elements #2497

Mellthas commented Jul 27, 2021

Mellthas commented Jul 29, 2021

Mellthas commented Aug 1, 2021

Mellthas commented Aug 3, 2021

bsweeney commented Sep 11, 2021 •

edited

Mellthas commented Sep 12, 2021

Mellthas commented Sep 12, 2021

Mellthas commented Sep 12, 2021

Mellthas commented Sep 12, 2021

Mellthas commented Sep 12, 2021

bsweeney commented Sep 12, 2021

bsweeney commented Sep 12, 2021

Mellthas commented Sep 12, 2021

Improve page break related to inline(-block) elements #2497

Improve page break related to inline(-block) elements #2497

Conversation

Mellthas commented Jul 27, 2021

Mellthas commented Jul 29, 2021

Mellthas commented Aug 1, 2021

Mellthas commented Aug 3, 2021

bsweeney commented Sep 11, 2021 • edited

Mellthas commented Sep 12, 2021

Mellthas commented Sep 12, 2021

Mellthas commented Sep 12, 2021

Mellthas commented Sep 12, 2021

Mellthas commented Sep 12, 2021

bsweeney commented Sep 12, 2021

bsweeney commented Sep 12, 2021

Mellthas commented Sep 12, 2021

bsweeney commented Sep 11, 2021 •

edited