[webkit-reviews] review requested: [Bug 84312] [Chromium] Added more params to Chromium-Webkit print api to implement auto fit to page functionality. : [Attachment 139405] Patch
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Mon May 14 10:53:18 PDT 2012
kmadhusu <kmadhusu at chromium.org> has asked for review:
Bug 84312: [Chromium] Added more params to Chromium-Webkit print api to
implement auto fit to page functionality.
https://bugs.webkit.org/show_bug.cgi?id=84312
Attachment 139405: Patch
https://bugs.webkit.org/attachment.cgi?id=139405&action=review
------- Additional Comments from kmadhusu <kmadhusu at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=139405&action=review
Addressed review comments. PTAL. Thanks.
>> Source/WebKit/chromium/public/WebFrame.h:468
>> + // TODO(kmadhusu at chromium.org): This is a temporary interface to avoid
the
>
> nit: webkit style doesn't use TODO(user). just use FIXME.
Fixed.
>> Source/WebKit/chromium/public/WebFrame.h:488
>> + virtual int printBegin(const WebSize& printContentSize,
>
> I suspect it would be better to bottle up these parameters into a
WebPrintParams
> struct. See for example WebPluginParams. Exclude the constrainToNode
parameter
> so that you can reuse WebPrintParams in the WebPlugin printBegin method.
Done.
>> Source/WebKit/chromium/public/WebPlugin.h:104
>> + // TODO(kmadhusu at chromium.org): Change the data type of
printScalingOption to
>
> nit: TODO(user) -> FIXME
>
> Use WebPrintParams here too.
Done.
More information about the webkit-reviews
mailing list