[webkit-reviews] review denied: [Bug 46550] [chromium] Changes to consolidate plugin zoom : [Attachment 68998] Patch

[bugzilla-daemon at webkit.org]

Darin Fisher (:fishd, Google) <fishd at chromium.org> has denied John Abd-El-Malek
<jam at chromium.org>'s request for review:
Bug 46550: [chromium] Changes to consolidate plugin zoom
https://bugs.webkit.org/show_bug.cgi?id=46550

Attachment 68998: Patch
https://bugs.webkit.org/attachment.cgi?id=68998&action=review

------- Additional Comments from Darin Fisher (:fishd, Google)
<fishd at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=68998&action=review

> WebKit/chromium/public/WebView.h:60
> +    static const double textSizeMultiplierRatio;

these will break the multi-dll build.  you need to use WEBKIT_API to export
these.

> WebKit/chromium/public/WebView.h:195
> +    virtual void fullFramePluginZoomLevelChanged(double zoomLevel) = 0;

this function seems pretty awkward and specialized to live on WebView.

> WebKit/chromium/public/WebView.h:197
> +    // Helper functions to conver between zoom level and zoom factor.  zoom

conver -> convert

> WebKit/chromium/public/WebView.h:199
> +    static double zoomLevelToZoomFactor(double zoomLevel);

these break the multi-dll build.  requires WEBKIT_API tags

> WebKit/chromium/public/WebViewClient.h:352
> +    // Zoom ----------------------------------------------------------------


add new line below here

> WebKit/chromium/public/WebViewClient.h:355
> +    virtual void zoomLimitsChanged(double minimumLevel, double maximumLevel)
{ }

add new line below here

> WebKit/chromium/src/WebViewImpl.cpp:1566
>      m_zoomLevel = zoomLevel;

this doesn't respect the min/max zoom levels