Bug 72801

Summary: Cannot print USPS shipping labels
Product: WebKit Reporter: Mike McNeil <mmcneil>
Component: PDFAssignee: mitz
Status: RESOLVED FIXED    
Severity: Normal CC: ap, mitz, mmcneil
Priority: P1 Keywords: InRadar, Regression
Version: 528+ (Nightly build)   
Hardware: Mac (Intel)   
OS: OS X 10.7   
URL: http://www.usps.com
Attachments:
Description Flags
Screen shot of URL
none
Make BuiltInPDFView execute JavaScript in PDF documents with a Doc object capable of printing the frame
none
Make BuiltInPDFView execute JavaScript in PDF documents with a Doc object capable of printing the frame
none
Make BuiltInPDFView execute JavaScript in PDF documents with a Doc object capable of printing the frame ap: review+

Description Mike McNeil 2011-11-19 09:44:41 PST
Created attachment 115952 [details]
Screen shot of URL

The process of printing a label on usps.com results in a label being downloaded in Safari 5.1. Does not currently work in Webkit.

This process used to result in a print dialogue, however that doesn't happen anymore in either Safari or Webkit.


https://sss-web.usps.com/cns/printPreparation.do
Comment 1 Alexey Proskuryakov 2011-11-21 16:46:22 PST
<rdar://problem/10463059>
Comment 2 Mike McNeil 2011-12-03 07:18:34 PST
Went through the entire label creation & printing process, address book bug 73657/73183 has been fixed. Still unable to get the label to print.

Anything I can do to assist or provide more information?

Version 5.1.2 (7534.52.7, r101927)

https://sss-web.usps.com/cns/printPreparation.do
Comment 3 Alexey Proskuryakov 2011-12-03 11:04:56 PST
Thank you. This cause of this problem is well understood, and its importance is also well understood.
Comment 4 mitz 2011-12-29 11:25:37 PST
Created attachment 120760 [details]
Make BuiltInPDFView execute JavaScript in PDF documents with a Doc object capable of printing the frame

Together with fixing bug 75232, this patch resolves the issue.
Comment 5 mitz 2011-12-29 11:27:47 PST
Created attachment 120761 [details]
Make BuiltInPDFView execute JavaScript in PDF documents with a Doc object capable of printing the frame
Comment 6 Alexey Proskuryakov 2011-12-31 11:14:41 PST
Comment on attachment 120761 [details]
Make BuiltInPDFView execute JavaScript in PDF documents with a Doc object capable of printing the frame

View in context: https://bugs.webkit.org/attachment.cgi?id=120761&action=review

Looks great.

Does this depend on bug 75232?

> Source/WebKit2/WebProcess/Plugins/PDF/BuiltInPDFView.cpp:838
> +    WebPage* page = frame->page();
> +    if (!page)

We have a check for pageless frame here, but not in BuiltInPDFView::initialize() or BuiltInPDFView::isActive(). Can page actually be null?

> Source/WebKit2/WebProcess/Plugins/PDF/BuiltInPDFView.cpp:841
> +    page->sendSync(Messages::WebPageProxy::PrintFrame(frame->frameID()), Messages::WebPageProxy::PrintFrame::Reply());

I'd have routed this through WebCore, for instance to make use of PageGroupLoadDeferrer once Chrome::print starts using it.

> Source/WebKit2/WebProcess/Plugins/PDF/BuiltInPDFView.h:35
> +typedef const struct OpaqueJSContext* JSContextRef;
> +typedef struct OpaqueJSValue* JSObjectRef;
> +typedef const struct OpaqueJSValue* JSValueRef;

:(

> Source/WebKit2/WebProcess/Plugins/PDF/BuiltInPDFView.h:139
> -    virtual void disconnectFromPage() { m_page = 0; }
> +    virtual void disconnectFromPage() { m_frame = 0; }

This is now a little bit surprising.
Comment 7 Alexey Proskuryakov 2011-12-31 11:15:15 PST
> Does this depend on bug 75232?

I now see the answer above.
Comment 8 mitz 2011-12-31 12:18:08 PST
Comment on attachment 120761 [details]
Make BuiltInPDFView execute JavaScript in PDF documents with a Doc object capable of printing the frame

View in context: https://bugs.webkit.org/attachment.cgi?id=120761&action=review

>> Source/WebKit2/WebProcess/Plugins/PDF/BuiltInPDFView.cpp:838
>> +    if (!page)
> 
> We have a check for pageless frame here, but not in BuiltInPDFView::initialize() or BuiltInPDFView::isActive(). Can page actually be null?

I’m quite certain it *cannot* be null in initialize(). I’m going to be paranoid and add a null check in isActive() rather than try to reason about lots of code outside this class. In this function, in particular, I don’t want to make any assumptions, in case Doc is ever extended to have something like setTimeout.

>> Source/WebKit2/WebProcess/Plugins/PDF/BuiltInPDFView.cpp:841
>> +    page->sendSync(Messages::WebPageProxy::PrintFrame(frame->frameID()), Messages::WebPageProxy::PrintFrame::Reply());
> 
> I'd have routed this through WebCore, for instance to make use of PageGroupLoadDeferrer once Chrome::print starts using it.

Good idea. I’ll do that.

>> Source/WebKit2/WebProcess/Plugins/PDF/BuiltInPDFView.h:35
>> +typedef const struct OpaqueJSValue* JSValueRef;
> 
> :(

You’ll thank me when JavaScriptCore public headers change and you don’t have to recompile all files that include this header!
Comment 9 mitz 2011-12-31 12:23:57 PST
Created attachment 120831 [details]
Make BuiltInPDFView execute JavaScript in PDF documents with a Doc object capable of printing the frame
Comment 10 mitz 2011-12-31 12:59:18 PST
Fixed in <http://trac.webkit.org/r103860>.