Last Comment Bug 355477 - map mouse buttons 8-9 to back/forward
: map mouse buttons 8-9 to back/forward
Product: Core
Classification: Components
Component: Widget: Gtk (show other bugs)
: Trunk
: x86 Linux
: -- normal (vote)
: mozilla1.9beta4
Assigned To: Olivier Crête
Depends on:
Blocks: 305896
  Show dependency treegraph
Reported: 2006-10-04 20:50 PDT by Olivier Crête
Modified: 2010-09-18 09:44 PDT (History)
10 users (show)
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---

patch against to map buttons 8-9 to back/forward (1.03 KB, patch)
2006-10-04 20:51 PDT, Olivier Crête
no flags Details | Diff | Splinter Review
Patch ported to trunk (4.47 KB, patch)
2008-02-10 17:36 PST, Olivier Crête
no flags Details | Diff | Splinter Review
Simplified patch against trunk (3.07 KB, patch)
2008-02-14 21:11 PST, Olivier Crête
roc: review+
roc: superreview+
mtschrep: approval1.9+
Details | Diff | Splinter Review

Description Olivier Crête 2006-10-04 20:50:54 PDT
User-Agent:       Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv: Gecko/20061004 Firefox/
Build Identifier: Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv: Gecko/20061004 Firefox/

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 2006-10-04 20:51:59 PDT
Created attachment 241282 [details] [diff] [review]
patch against to map buttons 8-9 to back/forward
Comment 2 Olivier Crête 2006-10-04 20:53:05 PDT
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 [:mwargers] (not working for Mozilla) 2007-08-21 07:25:28 PDT
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 2007-11-05 22:20:15 PST
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) 2008-01-06 09:50:44 PST
Definitely a worthwhile patch, esp. for Mighty Mouse etc., from an unexpected email address :). CONFIRMing.
Comment 6 Ben Bucksch (:BenB) 2008-01-06 09:59:21 PST
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 2008-02-10 17:36:17 PST
Created attachment 302501 [details] [diff] [review]
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 2008-02-10 17:37:46 PST
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) 2008-02-13 14:34:28 PST
Yeah, let's just use DispatchCommandKeyEvent. Rename it if you want.
Comment 10 Olivier Crête 2008-02-13 14:50:15 PST
The position doesn't matter? I'll attached an updated patch
Comment 11 Robert O'Callahan (:roc) (email my personal email if necessary) 2008-02-13 15:29:14 PST
I don't think it does.
Comment 12 Olivier Crête 2008-02-14 21:11:30 PST
Created attachment 303461 [details] [diff] [review]
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) 2008-02-14 21:32:50 PST
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] (use needinfo?) 2008-02-20 02:10:28 PST
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
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

Note You need to log in before you can comment on or make changes to this bug.