Closed
Bug 406474
Opened 17 years ago
Closed 16 years ago
Native GTK look for toolbar arrows
Categories
(Core :: Widget: Gtk, defect)
Tracking
()
RESOLVED
FIXED
mozilla1.9beta4
People
(Reporter: micmon, Assigned: twanno)
Details
Attachments
(2 files, 5 obsolete files)
10.50 KB,
image/png
|
Details | |
14.15 KB,
patch
|
roc
:
review+
roc
:
superreview+
beltzner
:
approval1.9+
|
Details | Diff | Splinter Review |
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•17 years ago
|
||
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•17 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•17 years ago
|
||
I think this has all been fixed in trunk as of today. Greg, can you check if something is still missing?
Comment 4•17 years ago
|
||
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•17 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•16 years ago
|
||
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•16 years ago
|
||
This is what the scroll arrows in menus will look like.
Attachment #302201 -
Flags: ui-review?(beltzner)
Assignee | ||
Comment 8•16 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•16 years ago
|
Attachment #302201 -
Attachment is patch: false
Updated•16 years ago
|
Attachment #302201 -
Attachment mime type: text/plain → image/png
Assignee | ||
Comment 9•16 years ago
|
||
And here is the patch to make it happen.
Assignee | ||
Comment 10•16 years ago
|
||
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•16 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?
Comment 12•16 years ago
|
||
(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•16 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•16 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.
Comment 15•16 years ago
|
||
(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•16 years ago
|
||
(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)
OK, use BACK/FORWARD then.
Assignee | ||
Comment 19•16 years ago
|
||
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+
Updated•16 years ago
|
Attachment #303289 -
Flags: approval1.9?
Comment 20•16 years ago
|
||
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+
Updated•16 years ago
|
Keywords: checkin-needed
Comment 21•16 years ago
|
||
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
Closed: 16 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.
Description
•