Bug 45993 – Convert printing to the new pagination model.

• Status: RESOLVED
• Resolution: FIXED
• Component: Layout and Rendering

[Dave Hyatt]

Columns are using the new pagination model, but printing isn't using it yet.  This patch switches printing over to use the new model.

[Dave Hyatt]

Created attachment 67941 [details]
Patch

[Dave Hyatt]

Created attachment 67943 [details]
Patch with comment typos fixed.

[WebKit Review Bot]

Attachment 67941 [details] did not pass style-queue:

Failed to run "['WebKitTools/Scripts/check-webkit-style']" exit_code: 1
WebCore/rendering/RenderView.h:132:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
Total errors found: 1 in 9 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[WebKit Review Bot]

Attachment 67943 [details] did not pass style-queue:

Failed to run "['WebKitTools/Scripts/check-webkit-style']" exit_code: 1
WebCore/rendering/RenderView.h:132:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
Total errors found: 1 in 9 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Dave Hyatt]

Created attachment 67944 [details]
Fix style issue.

[Dave Hyatt]

Comment on attachment 67944 [details]
Fix style issue.

Clearing review flag.  I have to get the layout test harness for printing updated to the new model too. :(

[Dave Hyatt]

Created attachment 67955 [details]
Patch

[Simon Fraser (smfr)]

Comment on attachment 67955 [details]
Patch

> Index: WebCore/ChangeLog
> ===================================================================
> --- WebCore/ChangeLog	(revision 67743)
> +++ WebCore/ChangeLog	(working copy)
> @@ -1,3 +1,47 @@
> [phone]  David Hyatt  <[e-mail]>
> +
> +        Reviewed by NOBODY (OOPS!).
> +
> +        https://bugs.webkit.org/show_bug.cgi?id=45993, convert printing to the new pagination model.

I'd like to see some discussion about what you did here.

> Index: WebCore/page/FrameView.h
> ===================================================================

> +    // FIXME: This method is retained because of embedded WebViews in AppKit.  When a WebView is embedded inside
> +    // some enclosing view with auto-pagination, no call happens to resize the view.  The new pagination model
> +    // needs the view to resize as a result of the breaks, but that means that the enclosing view has to potentially
> +    // resize around that view.  Auto-pagination uses the bounds of the actual view that's being printed to determine
> +    // the edges of the print operation, so the resize is necessary if the enclosing view's bounds depend on the
> +    // web document's bounds.
> +    // 
> +    // This is already a problem if the view needs to be a different size because of printer fonts or because of print stylesheets.
> +    // Mail/Dictionary work around this problem by using the _layoutForPrinting SPI
> +    // to at least get print stylesheets and printer fonts into play, but since WebKit doesn't know about the page offset or
> +    // page size, it can't actually paginate correctly during _layoutForPrinting.
> +    //
> +    // We can eventually move Mail to a newer SPI that would let them opt in to the layout-time pagination model,
> +    // but that doesn't solve the general problem of how other AppKit views could opt in to the better model.
> +    //
> +    // NO OTHER PLATFORM BESIDES MAC SHOULD USE THIS METHOD.
>      void adjustPageHeight(float* newBottom, float oldTop, float oldBottom, float bottomLimit);

I agree with renaming this method to make it more scarey-sounding. If I just looked at the header, I would not have seen this comment.

> Index: WebCore/rendering/RenderView.h
> ===================================================================

> +    // FIXME: Only used by embedded WebViews inside AppKit NSViews.  Find a way to remove.
> +    int m_bestTruncatedAt;
>      int m_truncatedAt;
> +    int m_truncatorWidth;
> +    IntRect m_printRect;
> +    bool m_forcedPageBreak;
> +    // End deprecated members.

Could these be moved into a struct just to keep them together?

> @@ -274,6 +292,7 @@ private:
>      bool m_disabled : 1;        // true if the offset and clip part of layoutState is disabled
>      bool m_didStart : 1;        // true if we did a push or disable
>      bool m_didEnd : 1;          // true if we popped or re-enabled
> +    bool m_didCreateLayoutState; // true if we actually made a layout state.

Put in the bitfield?

[Dave Hyatt]

Fixed in r67771.