[Webkit-unassigned] [Bug 58013] [Chromium] Needs eventSender.scalePageBy()

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=58013


Kentaro Hara <haraken at google.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #102490|1                           |0
        is obsolete|                            |




--- Comment #11 from Kentaro Hara <haraken at google.com>  2011-08-02 00:29:20 PST ---
(From update of attachment 102490)
View in context: https://bugs.webkit.org/attachment.cgi?id=102490&action=review

>> Source/WebKit/chromium/public/WebView.h:206
>> +    // Page scaling --------------------------------------------------------
> 
> nit: preserve two new lines above comment.

Done.

>> Source/WebKit/chromium/public/WebView.h:209
>> +    virtual void scalePageBy(float scaleFactor, float x, float y) = 0;
> 
> nit: use WebFloatPoint instead of raw "x" and "y"
> 
> this method sounds like it is related to setZoomLevel.  how does it differ?
> how is it related?  if related, then shouldn't it be in the "Zoom" section?
> 
> it is not obvious how zooming differs from scaling.  they sound like synonyms.

- I changed the signature to "void scalePage(float scaleFactor, WebPoint origin)". 

- I agreed that scalePage() is highly related to Zoom operations. Thus, I included scalePage() in the Zoom section, as you indicated.

- I am sorry but I cannot yet describe the difference in browser behavior between scalePage() and setZoomLevel(). However, scalePage() is not a synonym for setZoomLevel() since the implementations of these two methods are different. For example, at the level of WebView implementation, we can set any factor for scalePage() but can set the factor within a predefined range for setZoomLevel(). At the level of WebCore implementation, scalePage() manages |m_pageScaleFactor|, but setZoomLevel() manages |m_pageZoomFactor| and |m_textZoomFactor|.  I know this is not a good explanation for validating the existence of scalePage(), but scalePage() is a different thing from setZoomLevel().

>> Source/WebKit/chromium/public/WebView.h:211
>>      // Media ---------------------------------------------------------------
> 
> ditto

Done.

>> Source/WebKit/chromium/src/WebViewImpl.cpp:1819
>> +void WebViewImpl::scalePageBy(float scaleFactor, float x, float y)
> 
> nit: please insert new method definitions in the same order as they appear in the header, WebViewImpl.h.

Done.

>> Source/WebKit/chromium/src/WebViewImpl.h:388
>> +    void scalePageBy(float scaleFactor, float x, float y);
> 
> nit: please insert new method declarations in the same order as they appear in the interface, WebView.

Done.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.