406474 - Native GTK look for toolbar arrows

Status: RESOLVED FIXED

(Core :: Widget: Gtk, defect)

Description

[Michael Monreal [:micmon]]

Split out from bug 404493 (native GTK look for arrows in menus). It would be great if this native look would also be used for arrows on the toolbar (back/forward history, container/folder dropmarker)

Comment 1

[Grey Nicholson (they/them)]

The tab bar (the “List all tabs” menu and the scroll arrows), location bar and search box also still use non-native arrows (in 2007-12-02's trunk nightly). Similar greatness would ensue from applying the native look here too.

Comment 2

[Michael Monreal [:micmon]]

Right, the "list all tabs" is probably the same as the rest, the scroll arrows will need other arrows, though (the same as find next/find previous in the find bar at the bottom, see bug 399820).

Comment 3

[Michael Monreal [:micmon]]

I think this has all been fixed in trunk as of today. Greg, can you check if something is still missing?

Comment 4

[Grey Nicholson (they/them)]

Attachment: Labelled screenshot of the remaining problems

All the arrows seem to be correct now, except for those on the tab bar's scroll buttons, visible when there are many tabs open (the overflow UI). Those arrows are still filled triangles.

Also, the padding on the “List all tabs” button, though better than it was, is still asymmetric in Tardis fashion—it's bigger on the inside. *And* when hovering over it (thus revealing the asymmetric padding) the bottom row of pixels interrupts the bottom border of the tab bar.

(The drop-downs on Back and Forward don't behave in the same way as real Gnome drop-downs, but that's another bug.)

This is using the nightly from 2008-01-19T04.

Comment 5

[Michael Monreal [:micmon]]

I confirm the padding issues.

As for Back and Forward not having a "real Gnome" look, it's probably as good as it can be. Also, even native GTK apps like Evolution use other widgets for this, so they don't all look 100% the same. To "fix" the look, the dropdowns would need to be detached from the back/forward arrows, as if they are other toolbar elements.

Comment 6

[Teune van Steeg]

Attachment: scroll buttons drawn as menu scroll buttons

I'm currently looking at implementing native arrows for menus (when the menu is too large). A side effect of that would be that the tab bar scroll buttons would also look like those in a menu (see screenshot). Would that be OK?   

BTW, I think the issue of the misaligned all tabs button can better be fixed in another bug, like bug 415161.

Comment 7

[Teune van Steeg]

Attachment: native menu scroll buttons

This is what the scroll arrows in menus will look like.

Comment 8

[Teune van Steeg]

Comment on attachment 302155 [details]
scroll buttons drawn as menu scroll buttons

asking ui-review for the tab scroll arrows

Comment 9

[Teune van Steeg]

Attachment: patch

And here is the patch to make it happen.

Comment 10

[Teune van Steeg]

Attachment: implement native tab scroll arrows

It turns out that GTK has native tab scroll arrows which are part of the GtkNotebook widget, so we can just paint those.

Comment 11

[Michael Monreal [:micmon]]

Can those NS_THEME_TAB_SCROLLARROW_UP/DOWN also be used for the find bar (find previous, find next) instead of the arrow icons currently used?

Comment 12

[Grey Nicholson (they/them)]

(In reply to comment #10)
> It turns out that GTK has native tab scroll arrows which are part of the
> GtkNotebook widget, so we can just paint those.
> 

Note that those do a very different thing from Firefox's tab scroll arrows: Firefox's scroll the tab bar; GTK's change the selected tab (e.g. in Gedit).

It would be very confusing if Firefox's tab scroll arrows looked like GTK's but worked differently (e.g. bug 406020).

Comment 13

[Teune van Steeg]

(In reply to comment #11)
> Can those NS_THEME_TAB_SCROLLARROW_UP/DOWN also be used for the find bar (find
> previous, find next) instead of the arrow icons currently used?
> 

That is possible, however I can't tell how that would look compared to the arrows in native GTK apps (I thought there was a bug open for using arrows instead of arrow icons, but I can't find it right now). Note that the tab scroll arrows are not in a button and I suppose the arrows in the find bar should?

Comment 14

[Teune van Steeg]

(In reply to comment #12)
> Note that those do a very different thing from Firefox's tab scroll arrows:
> Firefox's scroll the tab bar; GTK's change the selected tab (e.g. in Gedit).
> 
There is a difference in behaviour, but the goal for the arrow is the same: bring tabs into view that are out of sight. I don't think the goal of the arrows in GTK is to select the tab next to the currently selected (why would the arrows appear only if there are too many tabs, in GTK as well as in Mozilla). Therefore my opinion is that we shouldn't invent our own appearance of some GTK widget in this case.

> It would be very confusing if Firefox's tab scroll arrows looked like GTK's but
> worked differently (e.g. bug 406020).
> 
In the case of the bug you mentioned, it is not that it works 'differently' but a feature for that widget available in GTK is missing in Mozilla's implementation.

Comment 15

[Grey Nicholson (they/them)]

(In reply to comment #14)
> There is a difference in behaviour, but the goal for the arrow is the same:
> bring tabs into view that are out of sight. I don't think the goal of the
> arrows in GTK is to select the tab next to the currently selected

That's how Gedit and Pidgin use them.

> (why would
> the arrows appear only if there are too many tabs, in GTK as well as in
> Mozilla).

Because a redundant button to switch to the adjacent tab is usually unnecessary, except when the adjacent tab is off the screen.

I agree it's a crap design, but it *does* appear to be the platform convention.

> my opinion is that we shouldn't invent our own appearance
> of some GTK widget in this case.

Yeah, I agree.

> > It would be very confusing if Firefox's tab scroll arrows looked like GTK's but
> > worked differently (e.g. bug 406020).
> > 
> In the case of the bug you mentioned, it is not that it works 'differently' but
> a feature for that widget available in GTK is missing in Mozilla's
> implementation.
> 

OK, bad example. My point was that two widgets that *look* the same should really *do* the same thing in all cases; otherwise it's inconsistent and confusing.

But I think in this case the native design is sufficiently rubbish that being inconsistent with its behaviour (despite looking the same) is probably a good thing. Just wanted to make sure you were aware of the discrepancy.

Comment 16

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

Why NS_THEME_TAB_SCROLLARROW_UP/DOWN? Shouldn't it be LEFT/RIGHT? Or possibly PREVIOUS/NEXT? Does the text direction actually affect the direction of these arrows?

Comment 17

[Teune van Steeg]

Attachment: implement native tab scroll arrows: using left/right

(In reply to comment #16)
> Why NS_THEME_TAB_SCROLLARROW_UP/DOWN? Shouldn't it be LEFT/RIGHT? Or possibly
> PREVIOUS/NEXT? Does the text direction actually affect the direction of these
> arrows?
> 
To answer your last question first: yes, the scrollbuttons are switched when the text direction is RTL. 
UP/DOWN was chosen because that is consistent with the class name of the buttons and because it leaves room to draw a different arrow when the text direction is RTL: IMHO we can use LEFT/RIGHT only when we always draw the arrow in the corresponding direction. The current patch implements that by using extra CSS selectors to make sure the correct arrow is drawn.

About PREVIOUS/NEXT: that sounds to me it would indicate that only one tab at a time would be scrolled (and selected). BACK/FORWARD would then be a better option.

Comment 18

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

OK, use BACK/FORWARD then.

Comment 19

[Teune van Steeg]

Attachment: implement native tab scroll arrows: using back/forward

Comment 20

[Mike Beltzner [:beltzner, not reading bugmail]]

Comment on attachment 303289 [details] [diff] [review]
implement native tab scroll arrows: using back/forward

a=beltzner for 1.9

Comment 21

[Reed Loden [:reed]]

Checking in browser/themes/gnomestripe/browser/browser.css;
/cvsroot/mozilla/browser/themes/gnomestripe/browser/browser.css,v  <--  browser.css
new revision: 1.178; previous revision: 1.177
done
Checking in gfx/public/nsThemeConstants.h;
/cvsroot/mozilla/gfx/public/nsThemeConstants.h,v  <--  nsThemeConstants.h
new revision: 1.25; previous revision: 1.24
done
Checking in layout/style/nsCSSKeywordList.h;
/cvsroot/mozilla/layout/style/nsCSSKeywordList.h,v  <--  nsCSSKeywordList.h
new revision: 3.100; previous revision: 3.99
done
Checking in layout/style/nsCSSProps.cpp;
/cvsroot/mozilla/layout/style/nsCSSProps.cpp,v  <--  nsCSSProps.cpp
new revision: 3.161; previous revision: 3.160
done
Checking in widget/src/gtk2/gtk2drawing.c;
/cvsroot/mozilla/widget/src/gtk2/gtk2drawing.c,v  <--  gtk2drawing.c
new revision: 1.79; previous revision: 1.78
done
Checking in widget/src/gtk2/gtkdrawing.h;
/cvsroot/mozilla/widget/src/gtk2/gtkdrawing.h,v  <--  gtkdrawing.h
new revision: 1.53; previous revision: 1.52
done
Checking in widget/src/gtk2/nsNativeThemeGTK.cpp;
/cvsroot/mozilla/widget/src/gtk2/nsNativeThemeGTK.cpp,v  <--  nsNativeThemeGTK.cpp
new revision: 1.144; previous revision: 1.143
done