RESOLVED FIXED 133005
[iOS] Create a UIPrintFormatter for WKWebView
https://bugs.webkit.org/show_bug.cgi?id=133005
Summary [iOS] Create a UIPrintFormatter for WKWebView
Andy Estes
Reported 2014-05-16 13:57:28 PDT
[iOS] Create a UIPrintFormatter for WKWebView
Attachments
Patch (34.66 KB, patch)
2014-05-16 14:29 PDT, Andy Estes
no flags
[iOS] Create a UIPrintFormatter for WKWebView (34.41 KB, patch)
2014-05-19 09:50 PDT, Andy Estes
no flags
[iOS] Create a UIPrintFormatter for WKWebView (34.53 KB, patch)
2014-05-19 11:21 PDT, Andy Estes
no flags
[iOS] Create a UIPrintFormatter for WKWebView (34.48 KB, patch)
2014-05-19 13:41 PDT, Andy Estes
no flags
Andy Estes
Comment 1 2014-05-16 14:29:51 PDT
Andy Estes
Comment 2 2014-05-19 09:50:31 PDT
Created attachment 231695 [details] [iOS] Create a UIPrintFormatter for WKWebView
Jonathan Wells
Comment 3 2014-05-19 11:12:11 PDT
Comment on attachment 231695 [details] [iOS] Create a UIPrintFormatter for WKWebView View in context: https://bugs.webkit.org/attachment.cgi?id=231695&action=review > Source/WebKit2/ChangeLog:11 > + returning the data to the UI process asynchronously. When UIPrintInteractionController later calls our "returning the data to the UI process asynchronously." I think is somewhat dangling. It isn't meant to modify "PDF". > Source/WebKit2/UIProcess/ios/WKPDFView.mm:31 > + Extra line here.
Andy Estes
Comment 4 2014-05-19 11:19:03 PDT
Comment on attachment 231695 [details] [iOS] Create a UIPrintFormatter for WKWebView View in context: https://bugs.webkit.org/attachment.cgi?id=231695&action=review >> Source/WebKit2/ChangeLog:11 >> + returning the data to the UI process asynchronously. When UIPrintInteractionController later calls our > > "returning the data to the UI process asynchronously." I think is somewhat dangling. It isn't meant to modify "PDF". I'll just add a new sentence that says "The PDF data will be returned to the UI process asynchronously."
Andy Estes
Comment 5 2014-05-19 11:21:30 PDT
Created attachment 231704 [details] [iOS] Create a UIPrintFormatter for WKWebView
Tim Horton
Comment 6 2014-05-19 13:30:06 PDT
Comment on attachment 231704 [details] [iOS] Create a UIPrintFormatter for WKWebView View in context: https://bugs.webkit.org/attachment.cgi?id=231704&action=review > Source/WebKit2/Shared/WebPreferencesStore.h:147 > + macro(ShouldPrintBackgrounds, shouldPrintBackgrounds, Bool, bool, DEFAULT_SHOULD_PRINT_BACKGROUNDS) \ oops! > Source/WebKit2/UIProcess/API/Cocoa/WKWebView.mm:1868 > + return { }; { }, really? > Source/WebKit2/UIProcess/API/Cocoa/WKWebView.mm:1892 > + if (!_page->process().connection()->waitForAndDispatchImmediately<Messages::WebPageProxy::DidFinishDrawingPagesToPDF>(_page->pageID(), didFinishLoadingTimeout)) { terrifying > Source/WebKit2/UIProcess/WKWebViewPrintFormatter.mm:35 > +@interface UIPrintFormatter (IPI) (Details) > Source/WebKit2/UIProcess/WKWebViewPrintFormatter.mm:100 > + CGContextTranslateCTM(context, 0, -CGPDFPageGetBoxRect(pdfPage, kCGPDFMediaBox).size.height); > + CGContextClipToRect(context, CGPDFPageGetBoxRect(pdfPage, kCGPDFCropBox)); I cannot remember; are you sure that CGPDFPageGetBoxRect falls back to the next box if the one you ask for isn't available? I guess this is fine if you always control _printedDocument.
Andy Estes
Comment 7 2014-05-19 13:34:36 PDT
Comment on attachment 231704 [details] [iOS] Create a UIPrintFormatter for WKWebView View in context: https://bugs.webkit.org/attachment.cgi?id=231704&action=review >> Source/WebKit2/UIProcess/API/Cocoa/WKWebView.mm:1892 >> + if (!_page->process().connection()->waitForAndDispatchImmediately<Messages::WebPageProxy::DidFinishDrawingPagesToPDF>(_page->pageID(), didFinishLoadingTimeout)) { > > terrifying More terrifying than a call to sendSync() (which would be the alternative here)?
Andy Estes
Comment 8 2014-05-19 13:39:05 PDT
Comment on attachment 231704 [details] [iOS] Create a UIPrintFormatter for WKWebView View in context: https://bugs.webkit.org/attachment.cgi?id=231704&action=review >> Source/WebKit2/UIProcess/API/Cocoa/WKWebView.mm:1868 >> + return { }; > > { }, really? Oops. That was from when this method returned a Vector<>. Will change. >> Source/WebKit2/UIProcess/WKWebViewPrintFormatter.mm:100 >> + CGContextClipToRect(context, CGPDFPageGetBoxRect(pdfPage, kCGPDFCropBox)); > > I cannot remember; are you sure that CGPDFPageGetBoxRect falls back to the next box if the one you ask for isn't available? I guess this is fine if you always control _printedDocument. Maybe I should just use the media box, or just not clip at all (thinking about it, clipping should be unnecessary, at least for the PDFs WebKit generates).
Andy Estes
Comment 9 2014-05-19 13:41:37 PDT
Created attachment 231719 [details] [iOS] Create a UIPrintFormatter for WKWebView
Tim Horton
Comment 10 2014-05-19 13:55:47 PDT
(In reply to comment #8) > (From update of attachment 231704 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=231704&action=review > > >> Source/WebKit2/UIProcess/API/Cocoa/WKWebView.mm:1868 > >> + return { }; > > > > { }, really? > > Oops. That was from when this method returned a Vector<>. Will change. > > >> Source/WebKit2/UIProcess/WKWebViewPrintFormatter.mm:100 > >> + CGContextClipToRect(context, CGPDFPageGetBoxRect(pdfPage, kCGPDFCropBox)); > > > > I cannot remember; are you sure that CGPDFPageGetBoxRect falls back to the next box if the one you ask for isn't available? I guess this is fine if you always control _printedDocument. > > Maybe I should just use the media box, or just not clip at all (thinking about it, clipping should be unnecessary, at least for the PDFs WebKit generates). Yeah, I think WebKit probably does enough clipping on the other end. If you ever end up handing PDFs from the web in here, though, you're going to want to do the right thing.
Andy Estes
Comment 11 2014-05-20 10:44:51 PDT
Comment on attachment 231719 [details] [iOS] Create a UIPrintFormatter for WKWebView I'm going to take a stab at making the page rect computation asynchronous.
Tim Horton
Comment 12 2014-05-21 12:53:52 PDT
Comment on attachment 231719 [details] [iOS] Create a UIPrintFormatter for WKWebView View in context: https://bugs.webkit.org/attachment.cgi?id=231719&action=review > Source/WebKit2/UIProcess/API/Cocoa/WKWebView.mm:1867 > + if (!_page->sendSync(Messages::WebPage::ComputePagesForPrintingAndStartDrawingToPDF(_page->mainFrame()->frameID(), printInfo, firstPage), Messages::WebPage::ComputePagesForPrintingAndStartDrawingToPDF::Reply(pageRects, totalScaleFactor))) Sam OK'd this.
WebKit Commit Bot
Comment 13 2014-05-21 13:40:06 PDT
Comment on attachment 231719 [details] [iOS] Create a UIPrintFormatter for WKWebView Clearing flags on attachment: 231719 Committed r169175: <http://trac.webkit.org/changeset/169175>
WebKit Commit Bot
Comment 14 2014-05-21 13:40:08 PDT
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.