[Webkit-unassigned] [Bug 35257] Expose WebFrame::setCanHaveScrollbars() to Chromium

[bugzilla-daemon at webkit.org]

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


Darin Fisher (:fishd, Google) <fishd at chromium.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #49331|review?                     |review-
               Flag|                            |




--- Comment #4 from Darin Fisher (:fishd, Google) <fishd at chromium.org>  2010-02-23 20:47:16 PST ---
(From update of attachment 49331)
> Index: ChangeLog

> +        Reviewed by NOBODY (OOPS!).
> +
> +        Expose WebFrame::setCanHaveScrollbars().  This allows a view
> +        which is being resized to not need scroll bars to ensure that
> +        they are not drawn.
> +
> +        Existing function setAllowsScrolling() was renamed
> +        setCanHaveScrollbars(), to be consistant with change 37159.
> +

^^^ please include a link to this bug.


> +        * WebKit/chromium/public/WebFrame.h:
> +          Expose setCanHaveScrollbars().
> +
> +        * WebKit/chromium/src/ChromeClientImpl.cpp:
> +          Rename setAllowsScrolling() to setCanHaveScrollbars()
> +
> +        * WebKit/chromium/src/WebFrameImpl.cpp:
> +          Rename setAllowsScrolling() to setCanHaveScrollbars()
> +
> +        * WebKit/chromium/src/WebFrameImpl.h:
> +          Rename setAllowsScrolling() to setCanHaveScrollbars()

^^^ this doesn't look like it is following the template produced
by prepare-ChangeLog


>          Add code that enables SquirrelFish Extreme (a.k.a JSCX, JSC JIT)
> -        in Android. It's disabled by default, but is enabled when the 
> +        in Android. It's disabled by default, but is enabled when the
>          enveronment variable ENABLE_JSC_JIT is set to true.
>          https://bugs.webkit.org/show_bug.cgi?id=34855
>  
> @@ -279,11 +302,11 @@
>          Reviewed by Simon Hausmann.
>  
>          First steps of the QtScript API.
> -        
> +
>          Two new classes were created; QScriptEngine and QScriptValue.
>          The first should encapsulate a javascript context and the second a script
>          value.
> -        
> +
>          This API is still in development, so it isn't compiled by default.
>          To trigger compilation, pass --qmakearg="CONFIG+=build-qtscript" to
>          build-webkit.
> @@ -655,7 +678,7 @@
>          Move GOwnPtr* from wtf to wtf/gtk
>          https://bugs.webkit.org/show_bug.cgi?id=31793
>  
> -        * GNUmakefile.am: Add JavaScriptCore/wtf/gtk to 
> +        * GNUmakefile.am: Add JavaScriptCore/wtf/gtk to
>            the include path.

^^^ I think it is best to leave existing entries in the ChangeLog unmodified.


> Index: WebKit/chromium/public/WebFrame.h
> ===================================================================
> --- WebKit/chromium/public/WebFrame.h	(revision 55153)
> +++ WebKit/chromium/public/WebFrame.h	(working copy)
> @@ -492,6 +492,9 @@ public:
>                                           float pageWidthInPixels,
>                                           float pageHeightInPixels) const = 0;
>  
> +    // If set to false, do not draw scrollbars on this frame's view.
> +    virtual void setCanHaveScrollbars(bool canHaveScrollbars) = 0;

Please move this to the "Geometry" section of WebFrame.  I would put this
method
just above the scrollOffset getter.  Also, the parameter does not require a
name
since it is clear that this method is setting a boolean attribute.


> +++ WebKit/chromium/src/WebFrameImpl.h	(working copy)

> -    void setAllowsScrolling(bool);
> +    void setCanHaveScrollbars(bool canHaveScrollbars);

^^^ ditto, do not name the parameter.

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