Bug 59597 – wrong page height is assumed during (regular HTML page) printing (concerning page breaks) when page is scaled down

• Component: Printing
• Status: NEW

[Christophe Saout]

When the document width is larger than the page logical width and the page is scaled down (see FrameView::forceLayoutForPagination), the printing still uses the old unscaled page height (which is now too small) and page breaks are occuring in the wrong place.

The page height is actually recomputed using the scale (using "root->setPageLogicalHeight(flooredPageLogicalWidth / pageSize.width() * pageSize.height()"), but the following call to "forceLayout();" fails to recompute the positions.

I traced it down to root->setNeedsLayoutAndPrefWidthsRecalc() not dirtying the whole tree. It just dirties the width of the root element, which, when its ::layout() is called, notices that the width is identical to its width from before and then does not relayout its children.

This is probably correct, as far as prefererred layout widths as such are concerned, but misses the fact that the page height has changed (and e.g. adjustForUnsplittableChild is never called to take the new page height into account).

I "fixed" it for our needs by applying this crude hack to make setNeedsLayoutAndPrefWidthsRecalc dirty the whole tree, so that the second forceLayout() goes through the hole tree (this should be like a forcePageHeightRecalc or sth.). I'd be happy to hear about suggestions how this should be done in a proper way. (I did not find any function to dirty the whole tree to force a full relayout, so I guess there might be some functionalty missing or at least I am missing something):


--- a/Source/WebCore/rendering/RenderObject.cpp
+++ b/Source/WebCore/rendering/RenderObject.cpp
@@ -2601,6 +2601,15 @@ void RenderObject::setNeedsBoundariesUpdate()
         renderer->setNeedsBoundariesUpdate();
 }
 
+void RenderObject::setNeedsLayoutAndPrefWidthsRecalc()
+{
+    setNeedsLayout(true);
+    setPreferredLogicalWidthsDirty(true);
+
+    for (RenderObject* child = firstChild(); child; child = child->nextSibling())
+        child->setNeedsLayoutAndPrefWidthsRecalc();
+}
+
 FloatRect RenderObject::objectBoundingBox() const
 {
     ASSERT_NOT_REACHED();
diff --git a/Source/WebCore/rendering/RenderObject.h b/Source/WebCore/rendering/RenderObject.h
index 75cc8a8..b1d4e9d 100644
--- a/Source/WebCore/rendering/RenderObject.h
+++ b/Source/WebCore/rendering/RenderObject.h
@@ -492,11 +492,7 @@ public:
     void setPreferredLogicalWidthsDirty(bool, bool markParents = true);
     void invalidateContainerPreferredLogicalWidths();
     
-    void setNeedsLayoutAndPrefWidthsRecalc()
-    {
-        setNeedsLayout(true);
-        setPreferredLogicalWidthsDirty(true);
-    }
+    void setNeedsLayoutAndPrefWidthsRecalc();
 
     void setPositioned(bool b = true)  { m_positioned = b;  }
     void setRelPositioned(bool b = true) { m_relPositioned = b; }

[Alexey Proskuryakov]

Could you please attach a test case and a screenshot of print preview (or output PDF, whichever is available on your platform)?

[Christophe Saout]

Created attachment 91441 [details]
pdf1.html without page breaks (default)

[Christophe Saout]

Created attachment 91442 [details]
pdf1.html with page breaks enabled in GTK+ port

[Christophe Saout]

Created attachment 91443 [details]
pdf2.html -> wrong page breaks

[Christophe Saout]

Created attachment 91444 [details]
pdf2.html -> issue "fixed" using my hack from original bug report

[Christophe Saout]

Created attachment 91445 [details]
pdf1.html (some HTML blocks without forced page width)

[Christophe Saout]

Created attachment 91446 [details]
pdf2.html - same page, but with am image added that forces a minimum page width

[Christophe Saout]

Created attachment 91447 [details]
code used for printing (using pywebkitgtk)

[Christophe Saout]

Ok, a few more details and a test case.

We wanted to add a simple PDF creator to an existing Python application. We thought Webkit was a rather awesome solution and went for the WebKit-GTK+ python bindings to use its PDF printing feature. We soon found out that it would be nice to have some control over the page breaks.

The GTK+ port by default doesn't do page breaks in printing. Not sure why, but it's trivially enabled and I have asked for this feature in a separate bug.

Note that in Source/WebCore/page/FrameView.cpp FrameView::forceLayoutForPagination is key to understanding the issue.  Therefore I've added some debug here to clarify the issue:

--- a/Source/WebCore/page/FrameView.cpp
+++ b/Source/WebCore/page/FrameView.cpp
@@ -2512,10 +2512,15 @@ void FrameView::forceLayoutForPagination(const FloatSize& pageSize, float maximu
         // FIXME: We are assuming a shrink-to-fit printing implementation.  A cropping
         // implementation should not do this!
         int docLogicalWidth = root->style()->isHorizontalWritingMode() ? root->docWidth() : root->docHeight();
+fprintf(stderr, "pageLogicalWidth = %f, pageLogicalHeight = %f, docLogicalWidth = %d\n", pageLogicalWidth, pageLogicalHeight, docLogicalWidth);
         if (docLogicalWidth > pageLogicalWidth) {
+fprintf(stderr, "docLogicalWidth > pageLogicalWidth!\n");
             flooredPageLogicalWidth = std::min<int>(docLogicalWidth, pageLogicalWidth * maximumShrinkFactor);
             if (pageLogicalHeight)
+{
+fprintf(stderr, "setting pageLogicalHeight = %d\n", (int)(flooredPageLogicalWidth / pageSize.width() * pageSize.height()));
                 root->setPageLogicalHeight(flooredPageLogicalWidth / pageSize.width() * pageSize.height());
+}
             root->setLogicalWidth(flooredPageLogicalWidth);
             root->setNeedsLayoutAndPrefWidthsRecalc();
             forceLayout();

Ok, so now running the test case:
> python print.py pdf1.html
produces an output.pdf (which is attached as test case1: output1.pdf)

pageLogicalWidth = 699,094482, pageLogicalHeight = 0,000000, docLogicalWidth = 699

As you can see, the GTK+ port is not passing the page height, therefore page breaks are ignored. This is not the issue I'm reporting here, and changed the port to pass the page height.  The issue I am reporting is not related to the GTK port (at least I'm 99.999% sure of that).

--- a/Source/WebKit/gtk/webkit/webkitwebframe.cpp
+++ b/Source/WebKit/gtk/webkit/webkitwebframe.cpp
@@ -756,7 +756,7 @@ static void begin_print_callback(GtkPrintOperation* op, GtkPrintContext* context
     float height = gtk_print_context_get_height(context);
     FloatRect printRect = FloatRect(0, 0, width, height);
 
-    printContext->begin(width);
+    printContext->begin(width, height);
 
     // TODO: Margin adjustments and header/footer support
     float headerHeight = 0;

Rerunning the test case now yields output2.pdf and everything is as it should be:

pageLogicalWidth = 699,094482, pageLogicalHeight = 979,462219, docLogicalWidth = 699

The page internally is 699 pixels wide and 979 pixels high.

Now I'm going to trigger the issue I am reporting.  I am adding an image to the page that is larger than the 699 pixels, forcing the large if() block in FrameView::forceLayoutForPagination to be entered, which tries to scale down the page again to fit it to the printing page.

Running  "python print.py pdf2.html" produces the output3.pdf:

pageLogicalWidth = 699,094482, pageLogicalHeight = 979,462219, docLogicalWidth = 744
docLogicalWidth > pageLogicalWidth!
setting pageLogicalHeight = 1042

The document is now 744 pixels wide, so the page is being rescale.  Because the page is now wider, it also implicitly gets higher than before.  In fact, the new height is computed as 1042 pixels.

But, if you look at the output3.pdf, the page breaks are now in the wrong place!  In fact, I traced it down to the layout code is still using the old page breaks at 979 pixels instead of the new one at 1042.

I then chased it down to the second ::forceLayout call below:

            root->setLogicalWidth(flooredPageLogicalWidth);
            root->setNeedsLayoutAndPrefWidthsRecalc();
            forceLayout();

I noticed that this forceLayout() does not relayout the document (as probably intended to) but bails out early, because it notices that the document width hasn't changed. (the new prefererred width of 744 is identical to the docLogicalWidth that was used before)

This means, that the vertical position of the blocks is also never recomputed using the new page height.

If I add in my hack from above, which goes through all the childs and sets "needsLayout" recursively to force a full re-layout, the resulting PDF is then correct again: see output4.pdf

So, essentially the problem boils down to the relayout in case the page is enlargened internally due to the scale-down feature does not take the need to relayout page breaks into account.

[Alexey Proskuryakov]

This works as expected in Safari 5.0.5 on Mac OS X, but I can reproduce the issue with WebKit2. It also likely occurs with most other ports.

[Alexey Proskuryakov]

<rdar://problem/9351814>