Bug 43271

Summary: Short documents may print a second blank page
Product: WebKit Reporter: mitz
Component: PrintingAssignee: mitz
Status: RESOLVED FIXED    
Severity: Normal CC: darin, dbates
Priority: P2 Keywords: InRadar
Version: 528+ (Nightly build)   
Hardware: Mac   
OS: OS X 10.6   
URL: data:text/html,
Attachments:
Description Flags
Use the page height as the viewport height when printing darin: review+

Description mitz 2010-07-30 14:43:48 PDT
When printing, elements that stretch to viewport height or whose height is specified as a percentage relative to the viewport have their heights computed relative to the WebView’s height. Often, such as when the WebView fills an entire browser window that takes up the entire height of the screen, those elements become taller than a page. One example, as in the URL, is the html and body elements in a quirks-mode document.

Patch (with cross-platform changes and fix for Mac) forthcoming
Comment 1 mitz 2010-07-30 14:44:23 PDT
<rdar://problem/8257783>
Comment 2 mitz 2010-07-30 21:08:59 PDT
Created attachment 63138 [details]
Use the page height as the viewport height when printing
Comment 3 Darin Adler 2010-07-30 21:57:54 PDT
Comment on attachment 63138 [details]
Use the page height as the viewport height when printing

> +void Frame::setPrinting(bool printing, float pageWidth, float pageHeight, float maximumShrinkRatio, AdjustViewSizeOrNot shouldAdjustViewSize)

I'd prefer const FloatSize& pageSize. Will that work?

>          // This magic is basically copied from khtmlview::print

Wow, you are leaving this comment!?

> +    void forceLayoutForPagination(float pageWidth, float pageHeight, float maximumShrinkFactor, Frame::AdjustViewSizeOrNot);

I'd prefer const FloatSize& pageSize here too. Make sense?

> +    void begin(float width, float height = 0);

What about here? FloatSize?

> -2010-07-30  Joseph Pecoraro  <joepeck@webkit.org>
> +2010-07-30  Dan Bernstein  <mitz@apple.com>

You removed Joe's change log entry!
Comment 4 mitz 2010-07-30 22:09:48 PDT
(In reply to comment #3)
> (From update of attachment 63138 [details])
> > +void Frame::setPrinting(bool printing, float pageWidth, float pageHeight, float maximumShrinkRatio, AdjustViewSizeOrNot shouldAdjustViewSize)
> 
> I'd prefer const FloatSize& pageSize. Will that work?

That’s how I did it at first. Then I confused myself into thinking that a FloatSize with a zero component (which would be used here to mean “use the unscaled visibleHeight()”) was bad. I am going to change back to const FloatSize&.

> 
> >          // This magic is basically copied from khtmlview::print
> 
> Wow, you are leaving this comment!?

Maybe not…

> 
> > +    void forceLayoutForPagination(float pageWidth, float pageHeight, float maximumShrinkFactor, Frame::AdjustViewSizeOrNot);
> 
> I'd prefer const FloatSize& pageSize here too. Make sense?

I am going to change this too.

> 
> > +    void begin(float width, float height = 0);
> 
> What about here? FloatSize?

I was frightened by a comment that said the even the width parameter needs to go away. Not going to change this now.

> 
> > -2010-07-30  Joseph Pecoraro  <joepeck@webkit.org>
> > +2010-07-30  Dan Bernstein  <mitz@apple.com>
> 
> You removed Joe's change log entry!

Oh noes!

Thanks for the review!
Comment 5 mitz 2010-07-30 23:00:37 PDT
Fixed in <http://trac.webkit.org/projects/webkit/changeset/64409>.
Comment 6 Daniel Bates 2010-07-31 00:10:12 PDT
(In reply to comment #5)
> Fixed in <http://trac.webkit.org/projects/webkit/changeset/64409>.

This change broke the Apple Windows build.
Comment 7 Daniel Bates 2010-07-31 00:10:23 PDT
Attempt to fix the build committed in changeset 64412 <http://trac.webkit.org/changeset/64412>.
Comment 8 mitz 2010-07-31 00:36:15 PDT
Thanks, Daniel!