355477 - map mouse buttons 8-9 to back/forward

Status: RESOLVED FIXED

(Core :: Widget: Gtk, defect)

Description

[Olivier Crête]

User-Agent:       Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv:1.8.0.7) Gecko/20061004 Firefox/1.5.0.7
Build Identifier: Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv:1.8.0.7) Gecko/20061004 Firefox/1.5.0.7

I'm attaching a simple patch that maps X mouse buttons 8-9 to the back/forward commands. The consensus amongst GTK+ and Qt seems to be that buttons 6-7 are for horizontal scrolling. Many new mouses have both 4 way scroll buttons and history control buttons, so we have to map them to different numbers. I'm using the appCommand infrastructure as used on windows.

Reproducible: Always

Comment 1

[Olivier Crête]

Attachment: patch against 1.5.0.7 to map buttons 8-9 to back/forward

Comment 2

[Olivier Crête]

I know the patch is against 1.5, but the code doesnt seem to be to different in 2.0 or HEAD

Comment 3

[Martijn Wargers (dead)]

Thanks for the patch!
But I don't think this kind of patch would be accepted for branch, since it's not a crash fix or security fix (or major problem).
Does this also need fixing on trunk? Could you make a patch for trunk? Thanks.

Comment 4

[Pierre Ossman]

Please do. This is a feature I've been missing from day one. Setting up imwheel on every machine is a bit of a pain.

Comment 5

[Ben Bucksch (:BenB)]

Definitely a worthwhile patch, esp. for Mighty Mouse etc., from an unexpected email address :). CONFIRMing.

Comment 6

[Ben Bucksch (:BenB)]

Patch submitter (no name given),

Thanks a lot for the patch!

Can you please update to latest trunk? The patch no longer applies. Once you did so, please request review from appropriate person using "Edit" on your patch. ccing reed for help (I can't see an obvious candidate for review).

I assume the user can still assign system-wide functions to these mouse buttons (e.g. switch window), and this feature won't interfere?

Ideally, it would be configurable within Mozilla. I'd prefer to map them to e.g. "switch to next tab on the right" and "close tab" instead of Back/Forward.

Comment 7

[Olivier Crête]

Attachment: Patch ported to trunk

Here is a patch ported to the trunk.
It shouldn't interfere with anything system-wide as I'd expect that to eat the event before we receive it. As for making it configurable, that would be nice, but I see this happening at a higher level (so that it would be available for windows/mac/etc..).
I'm asking roc for review since he's reviewed most of the recent patches to this file.

Comment 8

[Olivier Crête]

Also, I took some of the code used to other buttons event and put it in a more generic method, if its not required, we could just use the DispatchCommandKeyEvent() () method (and maybe renamed it to DispatchCommandEvent()..)

Comment 9

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

Yeah, let's just use DispatchCommandKeyEvent. Rename it if you want.

Comment 10

[Olivier Crête]

The position doesn't matter? I'll attached an updated patch

Comment 11

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

I don't think it does.

Comment 12

[Olivier Crête]

Attachment: Simplified patch against trunk

Here's a simplified patch as requested.. Btw, the reason for adding the position onto the event was that the windows version does it.

Comment 13

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

Comment on attachment 303461 [details] [diff] [review]
Simplified patch against trunk

When you ask for review, you should always specify a requestee. Otherwise people might not notice your request.

Comment 14

[Reed Loden [:reed]]

Checking in widget/src/gtk2/nsWindow.cpp;
/cvsroot/mozilla/widget/src/gtk2/nsWindow.cpp,v  <--  nsWindow.cpp
new revision: 1.260; previous revision: 1.259
done
Checking in widget/src/gtk2/nsWindow.h;
/cvsroot/mozilla/widget/src/gtk2/nsWindow.h,v  <--  nsWindow.h
new revision: 1.85; previous revision: 1.84
done