Bug 103286

Summary: PDFPlugin: <embed> and <object> PDFs affect their parent frame's page scale
Product: WebKit Reporter: Tim Horton <thorton>
Component: PDFAssignee: Tim Horton <thorton>
Status: RESOLVED FIXED    
Severity: Normal CC: andersca, ap, mitz, sam, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 103287    
Attachments:
Description Flags
patch
mitz: review+
followup style patch mitz: review+

Description Tim Horton 2012-11-26 13:34:19 PST 
Detecting whether or not a PDFPlugin is a full-page plugin or not is not as simple as checking isMainFrame(), because <embed> and <object> will cause PDFPlugin to live in the main frame, while still not being full-page. We also have to check that the main frame's document is a PluginDocument, and whether that PluginDocument's PluginWidget is our PDFPlugin's PluginView.
Comment 1 Tim Horton 2012-11-26 13:40:44 PST 
<rdar://problem/12752315>
Comment 2 Tim Horton 2012-11-26 15:10:06 PST 
Created attachment 176083 [details]
patch
Comment 3 Tim Horton 2012-12-01 23:52:56 PST 
Thanks, Dan!

http://trac.webkit.org/changeset/136316
Comment 4 Darin Adler 2012-12-03 09:51:26 PST 
Comment on attachment 176083 [details]
patch

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

> Source/WebKit2/WebProcess/Plugins/PDF/PDFPlugin.mm:664
> +    Document* document = webFrame()->coreFrame()->document();
> +    if (document->isPluginDocument())
> +        return (static_cast<PluginDocument*>(document)->pluginWidget() == pluginView());
> +
> +    return false;

Normally we do early return for the failure cases, not the main case, so it would be:

    if (!isPluginDocument)
        return false;

    return ......;

Also, the patch has extra parentheses in the return statement that we don’t do.

But I think this reads better with &&:

    return document->isPluginDocument() && static_cast<PluginDocument*>(document)->pluginWidget() == pluginView();
Comment 5 Tim Horton 2012-12-03 11:11:42 PST 
Created attachment 177290 [details]
followup style patch

Oooh, yes, much better. Posting a follow-up patch.
Comment 6 Tim Horton 2012-12-03 11:18:37 PST 
Followup style patch is http://trac.webkit.org/changeset/136422