409792 - Mousewheel scroll on tab bar should change tab in Suiterunner

Status: RESOLVED DUPLICATE of bug 643294

(Toolkit :: UI Widgets, defect)

Description

[Jens Hatlak (:InvisibleSmiley)]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; de-AT; rv:1.8.1.11) Gecko/20071128 SeaMonkey/1.1.7
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; de-AT; rv:1.8.1.11) Gecko/20071128 SeaMonkey/1.1.7

See Bug 356675: Switching the active tab through the use of the mousewheel was implemented for XPFE builds. That improvement was lost through the transition to Toolkit.

Reproducible: Always

Comment 1

[caught]

Attachment: updated patch for toolkit

just helping (I hope) by porting the previous patch forward. tested it and it works for me

Comment 2

[Jens Hatlak (:InvisibleSmiley)]

(In reply to comment #1)
> just helping (I hope) by porting the previous patch forward. tested it and it
> works for me

I can confirm that your patch works. :-) I think that the real file to be patched is /toolkit/content/widgets/tabbox.xml, though. Your patch seems to be against the packaged file (toolkit.jar).

So, I guess this is a toolkit bug/RFE then? Who needs to be consulted whether this can go in (given that adapting the patch to the real file is easy)?

Comment 3

[Philip Chee]

Changing to Toolkit->XUL Widgets

Comment 4

[[e-mail]]

Comment on attachment 321346 [details] [diff] [review]
updated patch for toolkit

>-          var next = startTab[aDir == -1 ? "previousSibling" : "nextSibling"];
>+          var next = aDir < 0 ? startTab.previousSibling : startTab.nextSibling;
>           if (!next && aWrap) {
>             next = aDir == -1 ? this.childNodes[this.childNodes.length - 1] :
>                                 this.childNodes[0];
Fortunately this particular line of code doesn't have a problem because aWrap is never true during a mouse scroll, but this function then calls _selectNewTab which also compares aFallbackDir == -1 which would fail in this case, so you should fix all the == -1 tests for consistency.

On a side note I wonder why toolkit fiddles about with childNodes instead of using the first/lastChild properties.

Comment 5

[Jens Hatlak (:InvisibleSmiley)]

It only occured to me now that if this is actually changing Toolkit files we'd either have to go for a global solution (i.e. one that FF people accept) or somehow make it SM-specific. As far as SM is concerned, this had already been implemented for XPFE builds, thus it's a regression. Looking at that XPFE bug 356675 you'll see in the initial comment that there are bugs for both core (bug 108524, WONTFIX'd) and FF (bug 281192, including patches against tabbox.xml and adding a pref) but that the latter was considered to be unneccessary for SM. I don't know how to proceed here. :-/

Comment 7

[Please Ignore This Troll (Account Disabled)]

another totally forgotten bug?
There is an extension, called Tab Wheel Scroll - maybe it's possible to copy some part of it's code to make the patch for this bug?

Comment 8

[[e-mail]]

Now that bug 484968 is fixed, the scrollable tabbar eats the wheel events.  But on Linux builds, you can still use the scroll wheel to change tabs, as long as you hover over the tab picker or the new or close tab buttons.

Comment 9

[Jens Hatlak (:InvisibleSmiley)]

(In reply to comment #8)
> Now that bug 484968 is fixed, the scrollable tabbar eats the wheel events.

Yeah, one of these cases where one groups wins and one loses, and I'm in the latter group though I was first (as far as SM is concerned). :-(

> But on Linux builds, you can still use the scroll wheel to change tabs, 
> as long as you hover over the tab picker or the new or close tab buttons.

Ah. I wonder whether that can be extended to work on all platforms, maybe with a modifier like Ctrl (Accel).

If everything goes wrong we can resort to providing an extension for this but I'd surely appreciate it if it was fixed in the application (so that the extension doesn't break in the future).

Comment 10

[Jens Hatlak (:InvisibleSmiley)]

Workaround: I just discovered that the following add-on works with both SM 2.0 and latest trunk (so quite probably also with 2.1 final):
<https://addons.mozilla.org/de/seamonkey/addon/tab-wheel-scroll/>

Comment 11

[Jens Hatlak (:InvisibleSmiley)]

So, since I originally filed this for SeaMonkey and it has been fixed for that now in bug 643294 (thanks Ratty!), I'm duping this bug to that one. IFF anyone's interested in having this for Firefox, please open a new bug for that if none exists yet.

Comment 12

[*I need to change my userid so it's less racist)]

Since the bug is alive in Firefox and there's no need to file a new one, just undo the status reset from "duplicate" to "new" for this bug.

Comment 13

[Jens Hatlak (:InvisibleSmiley)]

(In reply to comment #12)
> Since the bug is alive in Firefox and there's no need to file a new one, just
> undo the status reset from "duplicate" to "new" for this bug.

That's not the point. Some facts:
1. I reported the bug so I receive all bugmail for it
2. As the summary indicates, this was a request for Suiterunner (an intermediate version of SeaMonkey) and was only moved to Toolkit since SeaMonkey's move to it was what regressed it
3. The actual issue has only recently been fixed for SeaMonkey, just in another bug
4. Filing bugs is cheap.

So from a SeaMonkey perspective, both bugs really are the same, thus the duplicate resolution. If however this is transformed into a bug/request for Firefox, there will likely be a (lengthy) discussion whether or not to do this at all (for Firefox). To be honest, I'm not interested in that discussion at all, yet I'd receive all comments added to this bug. I'd have to filter out the whole bug in that case. For that reason, please take that to another bug.

The only valid reason for reopening this bug IMO would be if a decision would be made to actually do this for Firefox (with the discussion and decision happening elsewhere!) and fix it in Toolkit (so that SeaMonkey's overriding code could be removed). But even then this could be done in another bug.

Comment 14

[*I need to change my userid so it's less racist)]

lazy sunova...