[Webkit-unassigned] [Bug 75232] REGRESSION (WebKit2): Printing a subframe containing a PDF prints the on-screen view instead of the entire PDF document

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=75232


mitz at webkit.org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #120561|review+                     |review-
               Flag|                            |




--- Comment #5 from mitz at webkit.org  2011-12-26 19:14:16 PST ---
(From update of attachment 120561)
View in context: https://bugs.webkit.org/attachment.cgi?id=120561&action=review

Thanks for the review! I am going to work on this some more.

>> Source/WebKit2/Shared/PrintInfo.h:29
>> +#include <WebCore/FloatRect.h>
> 
> You could include FloatSize.h.

Indeed. At some point I was using a FloatRect here (which is really why I made the conversion).

>> Source/WebKit2/UIProcess/API/mac/WKView.mm:2734
>> +        // printing), this function should return nil.
> 
> Is this something you plan to address very soon? Otherwise, a bug number would be helpful.
> 
> What's the symptom here - will a single blank page be printed for non-printable documents?

I am not planning to address this immediately, since the UI process doesn’t know whether the frame contents are printable, and the symptom is quite benign (an single empty page). For what it’s worth, I think it’s strange that non-printable PDFs have been signaled only through a nil NSPrintOperation and not by disabling the print command.

>> Source/WebKit2/WebProcess/Plugins/PluginView.h:72
>> +    RetainPtr<CGPDFDocumentRef> pdfDocumentForPrinting() const { return m_plugin->pdfDocumentForPrinting(); }
> 
> I do not understand why we have to talk to WebCore here. Is this for <embed type="application/pdf">? But these end up in BuiltinPDFViews, too.
> 
> Eventually, there will be no CGPDFDocument parsing in WebProcess, so adding such plumbing across framework boundary is against the long term plan.

m_plugin is a WebKit::Plugin, not a WebCore object.

>> Source/WebKit2/WebProcess/WebPage/WebPage.cpp:2658
>> +    cropBox = CGRectIntersection(cropBox, CGPDFPageGetBoxRect(page, kCGPDFMediaBox));
> 
> This is suspicious - many PDFs don't have a crop box. See e.g. code in BuiltInPDFView::calculateSizes.

Interesting. I was trying to follow what PDFView does but I didn’t test thoroughly for compatibility.

>> Source/WebKit2/WebProcess/WebPage/WebPage.cpp:2668
>> +        CGContextTranslateCTM(context, widthDifference / 2, heightDifference / 2);
> 
> At least on screen, PDFs really dislike non-integer translation (see drawing code in BuiltinPDFView).

Also interesting!

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.