Native GTK look for toolbar arrows

RESOLVED FIXED in mozilla1.9beta4

Status

()

RESOLVED FIXED
11 years ago
11 years ago

People

(Reporter: micmon, Assigned: twanno)

Tracking

Trunk
mozilla1.9beta4
x86
Linux
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments, 5 obsolete attachments)

(Reporter)

Description

11 years ago
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)
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.
(Reporter)

Comment 2

11 years ago
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).
(Reporter)

Comment 3

11 years ago
I think this has all been fixed in trunk as of today. Greg, can you check if something is still missing?
Created attachment 298056 [details]
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.
(Reporter)

Comment 5

11 years ago
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.
(Assignee)

Comment 6

11 years ago
Created attachment 302155 [details]
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.
Assignee: nobody → twanno
Status: NEW → ASSIGNED
(Assignee)

Comment 7

11 years ago
Created attachment 302201 [details]
native menu scroll buttons

This is what the scroll arrows in menus will look like.
Attachment #302201 - Flags: ui-review?(beltzner)
(Assignee)

Comment 8

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

asking ui-review for the tab scroll arrows
Attachment #302155 - Flags: ui-review?(beltzner)
(Assignee)

Updated

11 years ago
Attachment #302201 - Attachment is patch: false
Attachment #302201 - Attachment mime type: text/plain → image/png
(Assignee)

Comment 9

11 years ago
Created attachment 302208 [details] [diff] [review]
patch

And here is the patch to make it happen.
(Assignee)

Comment 10

11 years ago
Created attachment 302461 [details] [diff] [review]
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.
Attachment #302155 - Attachment is obsolete: true
Attachment #302201 - Attachment is obsolete: true
Attachment #302208 - Attachment is obsolete: true
Attachment #302461 - Flags: superreview?(roc)
Attachment #302461 - Flags: review?(roc)
Attachment #302155 - Flags: ui-review?(beltzner)
Attachment #302201 - Flags: ui-review?(beltzner)
(Reporter)

Comment 11

11 years ago
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?
(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).
(Assignee)

Comment 13

11 years ago
(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?
(Assignee)

Comment 14

11 years ago
(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.
(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.
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?
(Assignee)

Comment 17

11 years ago
Created attachment 303029 [details] [diff] [review]
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.
Attachment #302461 - Attachment is obsolete: true
Attachment #303029 - Flags: superreview?(roc)
Attachment #303029 - Flags: review?(roc)
Attachment #302461 - Flags: superreview?(roc)
Attachment #302461 - Flags: review?(roc)
(Assignee)

Comment 19

11 years ago
Created attachment 303289 [details] [diff] [review]
implement native tab scroll arrows: using back/forward
Attachment #303029 - Attachment is obsolete: true
Attachment #303289 - Flags: superreview?(roc)
Attachment #303289 - Flags: review?(roc)
Attachment #303029 - Flags: superreview?(roc)
Attachment #303029 - Flags: review?(roc)
Attachment #303289 - Flags: superreview?(roc)
Attachment #303289 - Flags: superreview+
Attachment #303289 - Flags: review?(roc)
Attachment #303289 - Flags: review+
Attachment #303289 - Flags: approval1.9?
Comment on attachment 303289 [details] [diff] [review]
implement native tab scroll arrows: using back/forward

a=beltzner for 1.9
Attachment #303289 - Flags: approval1.9? → approval1.9+
Keywords: checkin-needed
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
Status: ASSIGNED → RESOLVED
Last Resolved: 11 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9beta4
You need to log in before you can comment on or make changes to this bug.