Floating images are always included in the current print page, even when they don't fit

[mflorea]

A common use case, both on the Web and on printed media, is to have text wrapped around an image (most of the time either on the left or on the right of the image). You can find many examples of this pattern on https://en.wikipedia.org for instance.

On the web this is achieved, most of the time, using the float CSS style. Here are two examples:

<p>text before</p>
<p><img style="float:right"/></p>
<p>text displayed on the left of the image</p>

<p>
  text displayed on..
  <img style="float:left"/>
  ..the right of the image
</p>

Paged.js doesn't handle this very well: when the image is near the end of a print page it gets cut / truncated or not displayed at all. The root problem seems to be that when looking for the overflow point to be used for splitting the content into print pages the floating elements are always skipped. In other words: paged.js thinks that floating elements never overflow, i.e. they always fit the current print page. This is obviously wrong.

If you look at the first example above, the image is inside a paragraph. The first issue we got is that since the image is floating the paragraph has 0 height so it never overflows. Moreover, since the paragraph is not considered a "container" its contents are copied without checking if they fit the print page. We "fixed" this by applying the same float style to the paragraph containing the image. This reduced the problem to the second example.

For the second example we debugged the behavior of Layout.findOverflow() and noticed that floating elements are explicitly skipped:

isFloat = styles.getPropertyValue("float") !== "none";
...
if (!br && !isFloat && isElement(node)) {
  // This node is overflowing so split the content before it.

Unfortunately there's no comment explaining why floating elements are skipped. So we tried to see what happens if we remove the check. With the following change:

diff --git a/src/chunker/layout.js b/src/chunker/layout.js
index ef24b89..76bb8d5 100644
--- a/src/chunker/layout.js
+++ b/src/chunker/layout.js
@@ -549,9 +549,6 @@ class Layout {
                                let bottom = Math.floor(pos.bottom);
 
                                if (!range && (left >= end || top >= vEnd)) {
-                                       // Check if it is a float
-                                       let isFloat = false;
-
                                        // Check if the node is inside a break-inside: avoid table cell
                                        const insideTableCell = parentOf(node, "TD", rendered);
                                        if (insideTableCell && window.getComputedStyle(insideTableCell)["break-inside"] === "avoid") {
@@ -560,7 +557,6 @@ class Layout {
                                                prev = insideTableCell.parentElement;
                                        } else if (isElement(node)) {
                                                let styles = window.getComputedStyle(node);
-                                               isFloat = styles.getPropertyValue("float") !== "none";
                                                skip = styles.getPropertyValue("break-inside") === "avoid";
                                                breakAvoid = node.dataset.breakBefore === "avoid" || node.dataset.previousBreakAfter === "avoid";
                                                prev = breakAvoid && nodeBefore(node, rendered);
@@ -615,7 +611,7 @@ class Layout {
                                                break;
                                        }
 
-                                       if (!br && !isFloat && isElement(node)) {
+                                       if (!br && isElement(node)) {
                                                range = document.createRange();
                                                range.selectNode(node);
                                                break;

all tests are passing (both npm test and npm run docker-specs). Moreover we also run our own automated tests with the modified paged.js and they also passed. So unfortunately the tests didn't provide any use case for skipping the floating elements when determining the overflow point. We also tried to see when this was introduced and apparently this is from the first commit (5 years ago) both on GitLab and on GitHub and the commit message doesn't say much.

Does anyone know why floating elements are skipped when finding the overflow?

[julientaq]

hi there.

it seems that when chrome changed the setup for the column break, this became to appear.

calling out to @fchasen because i think that may be quite useful for the next step of development.