281192 - Mousewheel scroll on tabbar should change tab

Status: RESOLVED FIXED

(Toolkit :: XUL Widgets, enhancement)

Description

[Vidar Haarr (not reading bugmail)]

GNOME 2.10 does this for all tabs - in pref dialogs, epiphany, gedit, etc. It
also enables it for the virtual desktop selector and other small things.

I see nothing wrong with doing this in Firefox as well, despite bug 108524.
However, I'm not reopening that bug, since it suggested a pref -- which I'm not.

Initial patch in a minute.

Comment 1

[Vidar Haarr (not reading bugmail)]

Attachment: version 0.1

First version

Comment 2

[Mike Connor [:mconnor]]

Comment on attachment 173475 [details] [diff] [review]
version 0.1

Making this global isn't right, still thinking about whether we should limit to
>= gtk2.6 or not.

Comment 3

[Vidar Haarr (not reading bugmail)]

mconnor: How would we do that ?

Comment 4

[Peter van der Woude [:Peter6]]

together with drag&drop tabs (FF1.5) it would nicely finish the taboptions.

10+ tabs squeezed on the tabbar results in poor readability/usability this
bug would be a major improvement.

(I'd personally love to se this for all OS)

Comment 5

[Peter van der Woude [:Peter6]]

my bad
forget my comment, I misread the bug

Comment 6

[Caleb]

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

Comment 7

[Martijn Wargers (dead)]

Attachment: version 0.2

Based on version 0.1
I have no idea how to make it >= gtk2.6 only.

Comment 8

[Dave Townsend [:mossop]]

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

Comment 9

[Vidar Haarr (not reading bugmail)]

Maybe we can use PR_FindFunctionSymbolAndLibrary or PR_FindSymbolAndLibrary and
lookup something GTK2.6 specific? I'm not sure what that would be though, but
here is a list of new symbols in GTK2.6: <http://www.gtk.org/api/2.6/gtk/ix05.html>.

Of course, this would require some C code as well as JS. In any case, I think we
should add a pref here:
'browser.tabs.mouseScrollChangesTab'
    0: Off.
    1: On.
    2: Behave according to platform.
Where 2 is default. If we don't add it here, we're sure to get an RFE for it
sooner or later anyway.

mconnor, think this could work? Where would we put the required C code for this?

Comment 10

[Vidar Haarr (not reading bugmail)]

Here is code that checks for GTK2.4, for reference:
<http://lxr.mozilla.org/mozilla/source/widget/src/gtk2/nsFilePicker.cpp#153>
(LoadSymbolsGTK24()).

Comment 11

[Vidar Haarr (not reading bugmail)]

Maybe we could (ahem) add a method like "GetToolkitVersion(out ACString
version)" to nsIToolkit ... :-P
Then we'd have to get a pointer to the current nsIToolkit implementation in JS
anyway, and I'm not sure it's exposed at all?

roc: Got a more sensible idea?

Comment 12

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

How about we just have an on/off pref and set it in platform-specific #ifdefs?
Would anyone really object to turning this on for all GTK versions?

Comment 13

[Brad Garcia]

Please be aware that this enhancement conflicts with Bug 194696 and possibly Bug
270101.

Comment 14

[Wayne Mery (:wsmwk)]

Is bug or fix linux only?
Does it also fix Bug 220397

Comment 15

[John Mellor (Jomel)]

(In reply to comment #14)
> Is bug or fix linux only?
No, hence OS:All. And I've successfully tested the patch on Windows.

> Does it also fix Bug 220397
No, quite the contrary, it causes different behaviour to that requested by bug
220397. If (when) this is fixed then we should wontfix 220397 since it would be
confusing and non-intuitive for the close button to behave differently from the
rest of the tabbar (plus switching tabs while the mouse is over the close button
is useful).

Comment 16

[Wayne Mery (:wsmwk)]

(In reply to comment #15)
> > Does it also fix Bug 220397
> No, quite the contrary, it causes different behaviour to that requested by bug
> 220397. 

So, it only scrolls tabs when mouse is actually over a tab - it doesn't permit
the entire tabbar?


>If (when) this is fixed then we should wontfix 220397 since it would be
> confusing and non-intuitive for the close button to behave differently from the
> rest of the tabbar 

I don't agree that this is a given. See below.


> plus switching tabs while the mouse is over the close button is useful.

personally I agree ... but it apparently depends on the user's perspective.

If main objective is decide which tab to close while scrolling through tabs ->
should allow scroll tabs over close button
If main objective is read a tab (and scroll the contents to help decide) if one
is done with it -> should allow scroll of page (not tabs) while over close button

Comment 17

[Vidar Haarr (not reading bugmail)]

Attachment: version 0.3

Like roc suggested in comment 12. As I remember it, mconnor wasn't decided on
whether it should be enabled for all GTK versions or not, like this patch does.

Comment 18

[John Mellor (Jomel)]

(In reply to comment #16)
> So, it only scrolls tabs when mouse is actually over a tab - it doesn't permit
> the entire tabbar?
No, it scrolls between tabs as long as the mouse is anywhere on the tab bar
(including the close button).

> > plus switching tabs while the mouse is over the close button is useful.
> personally I agree ... but it apparently depends on the user's perspective. [etc.]
Sure, but as both behaviours can be useful and one (close button behaves like
rest of tab bar) is fairly consistent, while one (close button does its own
thing and scrolls page) is unexpected, we may as well keep the interface
consistent. (bug 308215 is a different matter however and needs proper debate).

Comment 19

[John Mellor (Jomel)]

Surely we can get a simple but useful change like this in for 2.0, especially since the UI is one of the focuses of that release?

Comment 20

[Michael Ventnor]

Is the GTK version issue still a problem for Trunk? I'd like to make an updated fix for this.

Comment 21

[Nik Wegner]

If you have more tabs open than fit on the bar, it would be nice to be able to scroll to see other available tabs without switching through all of them in turn, like it currently does. Switching one by one would probably be glacial if any of the tabs is new, complex, and has to be drawn for the first time, too.

Comment 23

[Lars Strojny]

Are there plans to fix that in the upcoming 3.0 release?

Comment 25

[u294409]

#Nik Wegner, what about integration with GTK? Firefox tries to emulate GTK in the best way possible, so why it shouldn't behave like usual GTK app?

Comment 26

[Nik Wegner]

>what about integration with GTK?
This bug is marked OS:All. Maybe if you change it to OS:GNU you could fix it like that.

Comment 27

[u294409]

I want it to be integrated as well as possible.

Comment 32

[Vytautas Jakutis]

I think firefox on linux, as an app that uses GTK widgets, should look and feel like all GTK apps, so this scrolling must be implemented before the final 3.0 release.

Comment 33

[Mike Connor [:mconnor]]

Comment on attachment 195874 [details] [diff] [review]
version 0.3

Michael, if you want to do a new patch, I'll review.  This patch is well and truly obsolete now.

Comment 34

[Jens Hatlak (:InvisibleSmiley)]

(In reply to comment #20)
> Is the GTK version issue still a problem for Trunk? I'd like to make an updated
> fix for this.

The FF3 requirements page <http://www.mozilla.com/en-US/firefox/system-requirements.html> says GTK+ 2.10 or higher is required so this shouldn't be a concern anymore.

Given that SeaMonkey is also using Toolkit's tabbox.xml nowadays I'd love to see this fixed with a pref. That would allow us to fix bug 409792 by simply activating the feature for all platforms in SeaMonkey's default prefs file (in addition to porting the browser.js changes of course).

The only concern I have is that FF is already using the mouse wheel on the tab bar for scrolling through it when there are very many open tabs. SeaMonkey doesn't have that yet so it's currently not a concern for the suite.

Comment 35

[Micah Gersten]

Ubuntu bug:
https://bugs.launchpad.net/ubuntu/+source/firefox-3.0/+bug/200601

Comment 36

[Dão Gottwald [::dao]]

Attachment: patch

Comment 37

[Dão Gottwald [::dao]]

Attachment: patch v2

The event needs to be handled in the tabs binding, not the tabbox binding. Also, changing the scrollbox handler to phase="capturing" was unnecessary.

Comment 38

[Jens Hatlak (:InvisibleSmiley)]

The proposed patch only enables the functionality for GTK2 which is probably fine for FF but since it's shared (Toolkit) code and other applications may (actually: will, see comment 34) want to enable it unconditionally, is there a way to leave a backdoor open? If not with a pref, maybe with a symbol that would need to be defined by applications that want it that way? As far as I understand MDC, checking against a certain application (e.g. MOZ_SUITE) is discouraged so I'd suggest to check for the desire to have the feature instead.

Comment 39

[Mike Connor [:mconnor]]

How does this interact with the established behaviour of scrolling the tabstrip when we're overflowing?

Comment 40

[Dão Gottwald [::dao]]

When overflowed, the scrollbox consumes the event so that it won't reach the tabs binding.

Comment 41

[Mike Connor [:mconnor]]

That seems strange to me... it definitely seems like inconsistent UI to me, since as soon as I go to overflow, my interaction model breaks...

Comment 42

[Dão Gottwald [::dao]]

Then again, not supporting it at all in tabbrowser also breaks that model, just at a different level.
I agree that the mixture is strange, but I'm not sure that the alternatives are better.

Comment 43

[Dão Gottwald [::dao]]

Attachment: patch v2b

this excludes the tabbrowser tabbar

Comment 44

[Mike Connor [:mconnor]]

Comment on attachment 394634 [details] [diff] [review]
patch v2b

I think we should just do this, and not muck with the tabbrowser interaction model at this time.

Comment 45

[Dão Gottwald [::dao]]

http://hg.mozilla.org/mozilla-central/rev/4f3e5df9b2cd

Comment 46

[Michael Moravec]

I'm wondering how long you are able to speak about one bug...

Unfortunately this still does not work in nightly version of Firefox 4.0 (Mozilla/5.0 (X11; Linux i686; rv:2.0b3pre) Gecko/20100723 Minefield/4.0b3pre). Scrolling over tabs does not have any effect.