Closed Bug 281192 Opened 17 years ago Closed 12 years ago

Mousewheel scroll on tabbar should change tab

Categories

(Toolkit :: XUL Widgets, enhancement)

All
Linux
enhancement
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla1.9.3a1

People

(Reporter: vhaarr+bmo, Assigned: dao)

References

Details

Attachments

(2 files, 4 obsolete files)

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.
Attached patch version 0.1 (obsolete) — Splinter Review
First version
Attachment #173475 - Flags: review?(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.
Attachment #173475 - Flags: review?(mconnor) → review-
mconnor: How would we do that ?
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)
my bad
forget my comment, I misread the bug
*** Bug 250861 has been marked as a duplicate of this bug. ***
Attached patch version 0.2 (obsolete) — Splinter Review
Based on version 0.1
I have no idea how to make it >= gtk2.6 only.
*** Bug 308002 has been marked as a duplicate of this bug. ***
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?
Here is code that checks for GTK2.4, for reference:
<http://lxr.mozilla.org/mozilla/source/widget/src/gtk2/nsFilePicker.cpp#153>
(LoadSymbolsGTK24()).
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?
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?
Please be aware that this enhancement conflicts with Bug 194696 and possibly Bug
270101.
Is bug or fix linux only?
Does it also fix Bug 220397
(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).

(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


Attached patch version 0.3 (obsolete) — Splinter Review
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.
Attachment #173475 - Attachment is obsolete: true
Attachment #194094 - Attachment is obsolete: true
Attachment #195874 - Flags: review?(mconnor)
(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).
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?
Is the GTK version issue still a problem for Trunk? I'd like to make an updated fix for this.
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.
Duplicate of this bug: 410213
Are there plans to fix that in the upcoming 3.0 release?
Duplicate of this bug: 410401
#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?
>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.
I want it to be integrated as well as possible.
Duplicate of this bug: 417427
Duplicate of this bug: 419407
Duplicate of this bug: 420567
Duplicate of this bug: 426323
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 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.
Attachment #195874 - Attachment is obsolete: true
Attachment #195874 - Flags: review?(mconnor)
(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.
Assignee: vhaarr+bmo → nobody
Component: Tabbed Browser → XUL Widgets
Product: Firefox → Toolkit
QA Contact: tabbed.browser → xul.widgets
Attached patch patch (obsolete) — Splinter Review
Assignee: nobody → dao
Attachment #394356 - Flags: review?(enndeakin)
OS: All → Linux
Attached patch patch v2Splinter Review
The event needs to be handled in the tabs binding, not the tabbox binding. Also, changing the scrollbox handler to phase="capturing" was unnecessary.
Attachment #394356 - Attachment is obsolete: true
Attachment #394362 - Flags: review?(enndeakin)
Attachment #394356 - Flags: review?(enndeakin)
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.
How does this interact with the established behaviour of scrolling the tabstrip when we're overflowing?
When overflowed, the scrollbox consumes the event so that it won't reach the tabs binding.
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...
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.
Attachment #394362 - Flags: review?(enndeakin) → review?(mconnor)
Attached patch patch v2bSplinter Review
this excludes the tabbrowser tabbar
Attachment #394634 - Flags: review?(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.
Attachment #394634 - Flags: review?(mconnor) → review+
Attachment #394362 - Flags: review?(mconnor) → review-
http://hg.mozilla.org/mozilla-central/rev/4f3e5df9b2cd
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9.3a1
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.
You need to log in before you can comment on or make changes to this bug.