Bug 45993 - Convert printing to the new pagination model.
Summary: Convert printing to the new pagination model.
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Layout and Rendering (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC OS X 10.5
: P2 Normal
Assignee: Dave Hyatt
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-09-17 13:33 PDT by Dave Hyatt
Modified: 2010-09-17 19:14 PDT (History)
1 user (show)

See Also:


Attachments
Patch (24.77 KB, patch)
2010-09-17 13:36 PDT, Dave Hyatt
no flags Details | Formatted Diff | Diff
Patch with comment typos fixed. (24.77 KB, patch)
2010-09-17 13:39 PDT, Dave Hyatt
no flags Details | Formatted Diff | Diff
Fix style issue. (24.77 KB, patch)
2010-09-17 13:45 PDT, Dave Hyatt
no flags Details | Formatted Diff | Diff
Patch (29.08 KB, patch)
2010-09-17 14:58 PDT, Dave Hyatt
simon.fraser: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Dave Hyatt 2010-09-17 13:33:57 PDT
Columns are using the new pagination model, but printing isn't using it yet.  This patch switches printing over to use the new model.
Comment 1 Dave Hyatt 2010-09-17 13:36:07 PDT
Created attachment 67941 [details]
Patch
Comment 2 Dave Hyatt 2010-09-17 13:39:54 PDT
Created attachment 67943 [details]
Patch with comment typos fixed.
Comment 3 WebKit Review Bot 2010-09-17 13:42:20 PDT
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.
Comment 4 WebKit Review Bot 2010-09-17 13:43:25 PDT
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.
Comment 5 Dave Hyatt 2010-09-17 13:45:19 PDT
Created attachment 67944 [details]
Fix style issue.
Comment 6 Dave Hyatt 2010-09-17 14:41:27 PDT
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. :(
Comment 7 Dave Hyatt 2010-09-17 14:58:16 PDT
Created attachment 67955 [details]
Patch
Comment 8 Simon Fraser (smfr) 2010-09-17 15:51:02 PDT
Comment on attachment 67955 [details]
Patch

> Index: WebCore/ChangeLog
> ===================================================================
> --- WebCore/ChangeLog	(revision 67743)
> +++ WebCore/ChangeLog	(working copy)
> @@ -1,3 +1,47 @@
> +2010-09-17  David Hyatt  <hyatt@apple.com>
> +
> +        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?
Comment 9 Dave Hyatt 2010-09-17 19:14:11 PDT
Fixed in r67771.