Bug 44782 - [GTK] ScrollbarThemeGtk should respond to theme changes
Summary: [GTK] ScrollbarThemeGtk should respond to theme changes
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-08-27 11:49 PDT by Martin Robinson
Modified: 2010-08-31 15:22 PDT (History)
0 users

See Also:


Attachments
Patch (14.15 KB, patch)
2010-08-27 11:57 PDT, Martin Robinson
gns: review+
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-08-27 11:49:20 PDT
Scrollbar.cpp never expects scrollbars to change size, so even though theme changes currently trigger a re-layout, the scrollbar widget size does not change. We need to add some special code in ScrollbarThemeGtk which resizes all scrollbars when the theme specifies a different size.
Comment 1 Martin Robinson 2010-08-27 11:57:05 PDT
Created attachment 65746 [details]
Patch
Comment 2 Gustavo Noronha (kov) 2010-08-31 11:42:20 PDT
Comment on attachment 65746 [details]
Patch

 89     // If the theme changed, update the thickness of every scrollbar widget.
 90     // The platform-independent scrollbar code isn't yet smart enough to
 91     // get this information when it paints.
 92     HashSet<Scrollbar*>::iterator end = gScrollbars->end();
 93     for (HashSet<Scrollbar*>::iterator it = gScrollbars->begin(); it != end; ++it) {
 94         Scrollbar* scrollbar = (*it);

Saying 'If the theme changed' seems to imply that we're depending on a conditional, but if we're in this function it's because the theme has changed, so I'd suggest only stating what is being done. Looks good otherwise!
Comment 3 Martin Robinson 2010-08-31 14:45:11 PDT
> Saying 'If the theme changed' seems to imply that we're depending on a conditional, but if we're in this function it's because the theme has changed, so I'd suggest only stating what is being done. Looks good otherwise!

Okay. I'll fix this before landing. Thanks for the review. :)
Comment 4 Martin Robinson 2010-08-31 15:22:41 PDT
Committed r66540: <http://trac.webkit.org/changeset/66540>