Bug 68853 - [Chromium] Need a mechanism to query plugins for scaling to printable area or not.
Summary: [Chromium] Need a mechanism to query plugins for scaling to printable area or...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit API (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-09-26 17:26 PDT by Steve VanDeBogart
Modified: 2011-10-11 22:55 PDT (History)
3 users (show)

See Also:


Attachments
Add API to indicate if print scaling should be disabled. (5.53 KB, patch)
2011-09-26 17:30 PDT, Steve VanDeBogart
vandebo: commit-queue-
Details | Formatted Diff | Diff
Add API to indicate if print scaling should be disabled. (5.52 KB, patch)
2011-09-26 18:41 PDT, Steve VanDeBogart
fishd: review+
fishd: commit-queue-
Details | Formatted Diff | Diff
Add API to indicate if print scaling should be disabled. (5.55 KB, patch)
2011-10-11 12:58 PDT, Steve VanDeBogart
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Steve VanDeBogart 2011-09-26 17:26:33 PDT
The PDF format has a field to request that the output not be scaled to the printable area.  A decision about printing to the printable area vs. the entire page needs to be made before the printing flow starts.  The attached patch adds an accessor/API so that the plugins can indicate that print scaling should be disabled.
Comment 1 Steve VanDeBogart 2011-09-26 17:30:28 PDT
Created attachment 108765 [details]
Add API to indicate if print scaling should be disabled.

This patch depends on http://codereview.chromium.org/8041052/ which is still in review.
Comment 2 WebKit Review Bot 2011-09-26 17:37:15 PDT
Attachment 108765 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebKit/chromium/ChangeLog', u'Sourc..." exit_code: 1

Source/WebKit/chromium/public/WebFrame.h:453:  The parameter name "node" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/WebKit/chromium/src/WebFrameImpl.h:171:  The parameter name "node" adds no information, so it should be removed.  [readability/parameter_name] [5]
Total errors found: 2 in 7 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 3 Steve VanDeBogart 2011-09-26 18:41:21 PDT
Created attachment 108775 [details]
Add API to indicate if print scaling should be disabled.
Comment 4 Steve VanDeBogart 2011-09-29 11:54:13 PDT
Comment on attachment 108775 [details]
Add API to indicate if print scaling should be disabled.

Chromium part is in.  This can now land safely.
Comment 5 Darin Fisher (:fishd, Google) 2011-10-07 21:18:23 PDT
Comment on attachment 108775 [details]
Add API to indicate if print scaling should be disabled.

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

> Source/WebKit/chromium/public/WebFrame.h:451
> +    // If the frame or node is a plugin whose content indicates that printed

What does it mean for a frame to be a plugin?  Are you referring to the case of
a full-frame plugin, which has a PluginDocument?  Maybe say "If the frame contains
a full-frame plugin or the given node refers to a plugin, and the plugin's content
indicates that printed output..."

> Source/WebKit/chromium/src/WebFrameImpl.cpp:1404
> +    ASSERT(!frame()->document()->isFrameSet());

why do you have this assertion here?

> Source/WebKit/chromium/src/WebFrameImpl.cpp:1412
> +      return false;

nit: indentation
Comment 6 Steve VanDeBogart 2011-10-11 12:58:10 PDT
Created attachment 110562 [details]
Add API to indicate if print scaling should be disabled.

Address nits and remove unneeded Assert.
Comment 7 WebKit Review Bot 2011-10-11 13:00:25 PDT
Please wait for approval from fishd@chromium.org before submitting because this patch contains changes to the Chromium public API.
Comment 8 Steve VanDeBogart 2011-10-11 13:00:43 PDT
Comment on attachment 108775 [details]
Add API to indicate if print scaling should be disabled.

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

>> Source/WebKit/chromium/public/WebFrame.h:451
>> +    // If the frame or node is a plugin whose content indicates that printed
> 
> What does it mean for a frame to be a plugin?  Are you referring to the case of
> a full-frame plugin, which has a PluginDocument?  Maybe say "If the frame contains
> a full-frame plugin or the given node refers to a plugin, and the plugin's content
> indicates that printed output..."

Done.

>> Source/WebKit/chromium/src/WebFrameImpl.cpp:1404
>> +    ASSERT(!frame()->document()->isFrameSet());
> 
> why do you have this assertion here?

Copied from printBegin(), removed.

>> Source/WebKit/chromium/src/WebFrameImpl.cpp:1412
>> +      return false;
> 
> nit: indentation

Done.
Comment 9 WebKit Review Bot 2011-10-11 22:55:25 PDT
Comment on attachment 110562 [details]
Add API to indicate if print scaling should be disabled.

Clearing flags on attachment: 110562

Committed r97233: <http://trac.webkit.org/changeset/97233>
Comment 10 WebKit Review Bot 2011-10-11 22:55:30 PDT
All reviewed patches have been landed.  Closing bug.