Bug 103286 - PDFPlugin: <embed> and <object> PDFs affect their parent frame's page scale
Summary: PDFPlugin: <embed> and <object> PDFs affect their parent frame's page scale
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: PDF (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Tim Horton
URL:
Keywords: InRadar
Depends on:
Blocks: 103287
  Show dependency treegraph
 
Reported: 2012-11-26 13:34 PST by Tim Horton
Modified: 2012-12-03 11:18 PST (History)
5 users (show)

See Also:


Attachments
patch (3.57 KB, patch)
2012-11-26 15:10 PST, Tim Horton
mitz: review+
Details | Formatted Diff | Diff
followup style patch (1.56 KB, patch)
2012-12-03 11:11 PST, Tim Horton
mitz: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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