[Webkit-unassigned] [Bug 102323] Add Settings to disable custom scrollbars on main frame

[bugzilla-daemon at webkit.org]

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


Adam Barth <abarth at webkit.org> changed:

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




--- Comment #4 from Adam Barth <abarth at webkit.org>  2012-11-15 10:18:23 PST ---
(From update of attachment 174332)
View in context: https://bugs.webkit.org/attachment.cgi?id=174332&action=review

How do other mobile ports of WebKit solve this problem?

> Source/WebCore/page/FrameView.cpp:512
> +    if (m_frame && m_frame->page() && m_frame->settings()) {

The line below this one implies that m_frame can never be 0 here.

Rather than testing m_frame-page() for 0, we'd usually write this test as:

if (Settings* settings = m_frame->settings()) {
   ...
}

> Source/WebCore/page/Settings.h:657
> +        void setMainFrameCustomScrollbarDisabled(bool disabled) { m_mainFrameCustomScrollbarDisabled = disabled; }
> +        bool mainFrameCustomScrollbarDisabled() const { return m_mainFrameCustomScrollbarDisabled; }

We generally write settings in terms of "enabled" rather than "disabled".

>> Source/WebKit/chromium/src/WebSettingsImpl.cpp:65
>> +#if OS(ANDROID)
> 
> The normal approach is to have a separate patch in the Chromium tree doing this, since we try to avoid having #if OS() in WebKit.

Yeah, we shouldn't have OS(ANDROID) ifdefs in this code.  The reason to have Settings is so they can be configured by the embedder.

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