WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
84608
[Chromium] Added a chromium webkit public header file to support different web print scaling options.
https://bugs.webkit.org/show_bug.cgi?id=84608
Summary
[Chromium] Added a chromium webkit public header file to support different we...
kmadhusu
Reported
2012-04-23 10:16:02 PDT
To fix crbug.com/85132, I need a webkit public header file to provide different types of print scaling options. Actual fix for the bug(crbug.com/85132) include ppapi changes, chromium changes and webkit changes. In order to prevent any build breakage while committing the individual CL's, I would like to commit this header file first followed by the patch in
https://bugs.webkit.org/show_bug.cgi?id=84312
.
Attachments
Patch
(5.91 KB, patch)
2012-04-23 10:18 PDT
,
kmadhusu
no flags
Details
Formatted Diff
Diff
Patch
(3.82 KB, patch)
2012-04-23 10:20 PDT
,
kmadhusu
no flags
Details
Formatted Diff
Diff
Patch
(3.82 KB, patch)
2012-04-23 11:46 PDT
,
kmadhusu
no flags
Details
Formatted Diff
Diff
Show Obsolete
(2)
View All
Add attachment
proposed patch, testcase, etc.
kmadhusu
Comment 1
2012-04-23 10:18:00 PDT
Created
attachment 138374
[details]
Patch
kmadhusu
Comment 2
2012-04-23 10:20:22 PDT
Created
attachment 138375
[details]
Patch
WebKit Review Bot
Comment 3
2012-04-23 10:21:09 PDT
Please wait for approval from
abarth@webkit.org
,
dglazkov@chromium.org
,
fishd@chromium.org
,
jamesr@chromium.org
or
tkent@chromium.org
before submitting, as this patch contains changes to the Chromium public API. See also
https://trac.webkit.org/wiki/ChromiumWebKitAPI
.
Darin Fisher (:fishd, Google)
Comment 4
2012-04-23 11:26:46 PDT
Comment on
attachment 138375
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=138375&action=review
How do you plan to use this enum? It is often easier to review API changes when it is clear how the parts will fit together. This looks like an unused enum :-)
> Source/WebKit/chromium/public/WebPrintScalingOption.h:36 > +enum WebPrintScalingOption {
please fix naming:
http://trac.webkit.org/wiki/ChromiumWebKitAPI#Enums
kmadhusu
Comment 5
2012-04-23 11:46:42 PDT
Created
attachment 138395
[details]
Patch
kmadhusu
Comment 6
2012-04-23 11:47:37 PDT
Comment on
attachment 138375
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=138375&action=review
>> Source/WebKit/chromium/public/WebPrintScalingOption.h:36 >> +enum WebPrintScalingOption { > > please fix naming: >
http://trac.webkit.org/wiki/ChromiumWebKitAPI#Enums
I assume you meant to change the enum value names. Done.
kmadhusu
Comment 7
2012-04-23 11:48:28 PDT
Sorry I didn't state it clearly in my description. You can see the usage of this enum in the latest patch uploaded in
https://bugs.webkit.org/show_bug.cgi?id=84312
. And you can also find the usage of this enum in the following chromium side CL's.
http://codereview.chromium.org/10083059/
http://codereview.chromium.org/10168013/
Thanks.
kmadhusu
Comment 8
2012-04-23 11:50:44 PDT
Patch in
bug 84312
uses WebPrintFitToPrintableArea enum value. After addressing all the review comments in this header file, I will update the patch in
bug 84312
accordingly.
kmadhusu
Comment 9
2012-04-25 18:39:07 PDT
ping
Darin Fisher (:fishd, Google)
Comment 10
2012-04-27 11:07:03 PDT
Comment on
attachment 138395
[details]
Patch OK, R=me Normally, it is better to include a change like this with the patch that actually requires it.
kmadhusu
Comment 11
2012-04-27 11:18:20 PDT
Thanks for reviewing the patch. I understand your concern. I am just trying to avoid chrome canary build breakages. By landing this header file first, it easier for me to land the individual patches. Will it help if I add more information in the ChangeLog file? Can you also please review the other webkit patch (
https://bugs.webkit.org/show_bug.cgi?id=84312
) which actually uses this enum to make sure you are fine with the workflow?
WebKit Review Bot
Comment 12
2012-04-27 12:01:54 PDT
Comment on
attachment 138395
[details]
Patch Clearing flags on attachment: 138395 Committed
r115464
: <
http://trac.webkit.org/changeset/115464
>
WebKit Review Bot
Comment 13
2012-04-27 12:01:59 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.
Top of Page
Format For Printing
XML
Clone This Bug