Bug 45046 - [GTK] ScrollbarThemeGtk should be enabled for interior frame scrollbars
Summary: [GTK] ScrollbarThemeGtk should be enabled for interior frame scrollbars
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKitGTK (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC Linux
: P3 Normal
Assignee: Nobody
URL:
Keywords: Gtk
Depends on:
Blocks:
 
Reported: 2010-09-01 10:38 PDT by Martin Robinson
Modified: 2010-09-07 03:01 PDT (History)
1 user (show)

See Also:


Attachments
Enable ScrollbarThemeGtk (33.60 KB, patch)
2010-09-01 10:55 PDT, Martin Robinson
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Martin Robinson 2010-09-01 10:38:06 PDT
ScrollbarThemeGtk should be enabled for interior frame scrollbars. This will specialize ScrollbarGtk.cpp to only apply to main-frame scrollbars and remove all code in that unit for interior frame scrollbars. This means that interior frame scrollbars will work just as they do for all other ports and greatly reduces the code complexity.

Further benefits:
For WebKit2 clicks will not have to be routed through GTK+ and can simply go straight to WebCore. Additionally, it will allow us to add a switch (if we desire) to toggle the behavior of main frame scrollbars (for instance, if we wish to pass some failing layout tests).
Comment 1 Martin Robinson 2010-09-01 10:55:12 PDT
Created attachment 66236 [details]
Enable ScrollbarThemeGtk
Comment 2 Gustavo Noronha (kov) 2010-09-06 13:05:14 PDT
Comment on attachment 66236 [details]
Enable ScrollbarThemeGtk

 183     // FIXME: Restructure the ScrollView abstraction so that we do not have to
 184     // copy this verbatim from ScrollView.cpp. Until then, we should make sure this
 185     // is kept in sync.
 186     bool needsUpdate = false;

Yeah =(. I like that we are being able to further isolate the more specific stuff into their own smaller compartments, though.
Comment 3 Martin Robinson 2010-09-06 13:26:54 PDT
Wow. Thanks for the review. That's a wrap for scrollbar stuff. Enabling / disabling the current main frame behavior should only be a couple-line change if we need to do it for WebKit2.
Comment 4 Martin Robinson 2010-09-06 14:04:15 PDT
Comment on attachment 66236 [details]
Enable ScrollbarThemeGtk

Clearing flags on attachment: 66236

Committed r66848: <http://trac.webkit.org/changeset/66848>
Comment 5 Martin Robinson 2010-09-06 14:04:19 PDT
All reviewed patches have been landed.  Closing bug.
Comment 6 Philippe Normand 2010-09-07 03:01:36 PDT
It seems testwebview started to crash after this patch landed:
http://webkit-bots.igalia.com/amd64/svn_66871.core-when_1283853203-_-who_testwebview-_-why_11.7435.trace.html

Looking into it