263444 - GTK2 Rendering does not support special theme

Status: RESOLVED FIXED

(Core Graveyard :: GFX: Gtk, defect)

Description

[Julian Sheldahl]

User-Agent:       Mozilla/5.0 (X11; U; Linux i686; rv:1.7.3) Gecko/20040914 Firefox/0.10
Build Identifier: Mozilla/5.0 (X11; U; Linux i686; rv:1.7.3) Gecko/20040914 Firefox/0.10

The GTK rendering that I have seen this far is nothing short of amazing.
However, I recently noticed how Firefox and Thunderbird emulate the scrollbars
in the GTK theme Aluminum Alloy (at http://art.gnome.org/themes/gtk2/614/)
incorrectly. The bottom scrollbars are supposed to have an up and down button,
similar to how Mac OS works. The GTK integration lets paints the up arrow (at
the bottom of the screen), but does not treat it as a button, but another part
of the slider.

This isn't a huge deal, but I know that desktop integration is important for
Mozilla.

THANK YOU FOR ALL THAT YOU DO!

Reproducible: Always
Steps to Reproduce:
1. Drag scroll bar to bottom of page in Firefox
2. Drag scroll bar to end of document in another GTK application.
3. Compare.

Actual Results:  
I noticed the button covered up by the slider on the vertical scrollbar.

Expected Results:  
Treated the bottom "up" button as a button. Not slid into the button.

Comment 1

[Julian Sheldahl]

Attachment: Screenshot of the problem

Comment 2

[nulldomain]

I think this is invalid, since it's the theme causing it.

Comment 3

[Reinout van Schouwen]

The viewport scrollbar does not honor the GtkScrollbar::has_backward_stepper and
GtkScrollbar::has_secondary_backward_stepper theme directives (necessary to
control the placement of scrollbar arrows).

Related to bug 160085 ?

Comment 4

[Gervase Markham [:gerv]]

This is an automated message, with ID "auto-resolve01".

This bug has had no comments for a long time. Statistically, we have found that
bug reports that have not been confirmed by a second user after three months are
highly unlikely to be the source of a fix to the code.

While your input is very important to us, our resources are limited and so we
are asking for your help in focussing our efforts. If you can still reproduce
this problem in the latest version of the product (see below for how to obtain a
copy) or, for feature requests, if it's not present in the latest version and
you still believe we should implement it, please visit the URL of this bug
(given at the top of this mail) and add a comment to that effect, giving more
reproduction information if you have it.

If it is not a problem any longer, you need take no action. If this bug is not
changed in any way in the next two weeks, it will be automatically resolved.
Thank you for your help in this matter.

The latest beta releases can be obtained from:
Firefox:     http://www.mozilla.org/projects/firefox/
Thunderbird: http://www.mozilla.org/products/thunderbird/releases/1.5beta1.html
Seamonkey:   http://www.mozilla.org/projects/seamonkey/

Comment 5

[Reinout van Schouwen]

Confirming, then. This bug blocks bug 160085.

Comment 6

[Jonathan Haas]

Currently Mozilla does not honour the following rules in the gtkrc file:

#show additional down/right arrow at top/left
  GtkScrollbar   ::has-secondary-forward-stepper = 1

#show additional up/left arrow at bottom/right
  GtkScrollbar   ::has-secondary-backward-stepper = 1

#hide normal up/left arrow (at top/left)
  GtkScrollbar   ::has-backward-stepper = 0

#hide normal down/right arrow (at bottom/right)
  GtkScrollbar   ::has-forward-stepper = 0

Mozilla should respect this setting and apply the correct CSS display-value to the scrollbarbuttons.

Comment 7

[Mike Connor [:mconnor]]

Its not just a matter of CSS, its a matter of implementing scrollbars that support the functionality.  This is a core bug, in any case.

Comment 8

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Our scrollbars do support this, actually. They need to be hooked up here somehow.

Comment 9

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Just need to return one of these:
http://lxr.mozilla.org/mozilla/source/widget/public/nsILookAndFeel.h#205
from nsILookAndFeel::GetMetric(nsILookAndFeel::eMetric_ScrollArrowStyle). Shouldn't be hard.

Comment 10

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Namely, from here:
http://lxr.mozilla.org/mozilla/source/widget/src/gtk2/nsLookAndFeel.cpp#464

Comment 11

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Is
            settings = gtk_settings_get_default ();
            g_object_get (settings, "GtkScrollbar::has-secondary-forward-stepper", &secondary_forward, NULL);

the right way to access these settings?

Comment 12

[Christian Persch (GNOME) (away; not receiving bug mail)]

(In reply to comment #11)
> Is
>             settings = gtk_settings_get_default ();
>             g_object_get (settings,
> "GtkScrollbar::has-secondary-forward-stepper", &secondary_forward, NULL);
> 
> the right way to access these settings?
> 

Those are style properties, so you need to use gtk_widget_style_get on a GtkScrollbar widget.

Comment 13

[Jonathan Haas]

Some (very incomplete) sample code that may help you:

     GtkWidget *scrollbar;
//create a new scrollbar
     scrollbar = gtk_vscrollbar_new(NULL);
//replace "widget" with a soething you want to add the scollbar
//it does not work without this line. (WHY?)
     gtk_container_add (GTK_CONTAINER (widget), scrollbar);
     gboolean has_secondary_forward_stepper;
 
     gtk_widget_style_get (scrollbar,"has-secondary-forward-stepper",
                                     &has_secondary_forward_stepper,NULL);
  

    if(has_secondary_forward_stepper == TRUE)
//      show the button

Comment 14

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Attachment: fix

This does the trick.

Comment 15

[Brian Ryner (not reading)]

Comment on attachment 219959 [details] [diff] [review]
fix

Looks good ... I think the Mac special casing was fairly unnecessary since these bindings won't be getting instantiated on Mac anyway.

Comment 16

[Robert O'Callahan (:roc) (email my personal email if necessary)]

checked in.

Comment 17

[[e-mail]]

(In reply to comment #15)
>I think the Mac special casing was fairly unnecessary since
>these bindings won't be getting instantiated on Mac anyway.
Oh yes they will. The Modern theme uses them for instance.

Comment 18

[Robert O'Callahan (:roc) (email my personal email if necessary)]

*** Bug 206554 has been marked as a duplicate of this bug. ***

Comment 19

[Adam Guthrie]

*** Bug 313172 has been marked as a duplicate of this bug. ***

Comment 20

[Adam Guthrie]

*** Bug 344522 has been marked as a duplicate of this bug. ***

Comment 21

[Adam Guthrie]

Comment on attachment 219959 [details] [diff] [review]
fix

There have been no known regressions from this patch since its landing, and it seems like a commonly-requested feauture by Firefox users on KDE.

Comment 22

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Does roc think this should go on 1.8.1?

Comment 23

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Yes, I think it's worth having.

Comment 24

[Mike Connor [:mconnor]]

Comment on attachment 219959 [details] [diff] [review]
fix

a=mconnor on behalf of drivers for checkin by Wednesday, July 19th

Comment 25

[jag (Peter Annema)]

This broke the Modern theme in SeaMonkey on Mac.

The old enum values (well, the first two) were equal to Mac's enum values. They no longer are, so we need to update the Mac L&F code to map. Patch in bug 339576.

Comment 26

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Thanks. This should not be checked in until that patch lands on branch --- if it ever does.