1052947 - WidgetMouseEvent::e4thButtonFlag and e5thButtonFlag or windows WM_XBUTTONDOWN / XBUTTON1 mouse events are always handeled as Browser: Forward and Browser:Back and there is no option to disable this functionality in about:config or anywhere else

Status: RESOLVED FIXED

(Core :: Widget: Win32, defect)

Description

[Steve Collins]

User Agent: Mozilla/5.0 (Windows NT 6.3; WOW64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/33.0.1750.154 Safari/537.36

Steps to reproduce:

press 4th or 5th mouse button


Actual results:

Browser api handles as Browser:Forward and Browser:Back requests


Expected results:

there should be a about:config option to disable this forward/back functionality so that pressing mouse buttons 4 and 5 (MK_XBUTTON1 and MK_XBUTTON2 under windows) do nothing.

Comment 1

[:Gijs (he/him)]

From casually looking through MXR, it's not obvious to me that this is Firefox itself calling forward/back when the 4th/5th mouse button is clicked... certainly I don't see where that's happening, neither when looking for callers of forward/back nor when looking for uses of the mouse button constants. Can you elaborate on where you believe this issue comes from, and if you're sure it's not a feature of your mouse software and/or an add-on you have installed?

Comment 2

[:Gijs (he/him)]

In particular, AFAICT windows actually sends WM_XBUTTONUP/DOWN messages for these buttons, and we don't even listen for those, so I'm pretty sure this isn't Firefox itself. Check if you can reproduce in safe mode (Help > Restart with add-ons disabled).

Please reopen if you have more information on why you believe this is Firefox's fault.

Comment 3

[Steve Collins]

This is in response to bugs like this: Bug 360731 - Back and Forward buttons on mouse no longer work 
which were given a priority to fix, but no thought was given to those of us that don't want that functionality.

Comment 4

[:Gijs (he/him)]

(In reply to Steve Collins from comment #3)
> This is in response to bugs like this: Bug 360731 - Back and Forward buttons
> on mouse no longer work 
> which were given a priority to fix, but no thought was given to those of us
> that don't want that functionality.

The patch there didn't change how we handle the buttons, as far as I can tell, it changed how we handle external commands sent to us by your mouse software through windows (et al.). If you don't want those buttons to send those commands when you hit those buttons, it seems like that software should be updated/configured to not do that.

Olli, can you check if I'm misunderstanding how this works?

Comment 5

[Olli Pettay [:smaug][[e-mail]](vacation until July 14)]

Masayuki has played with mouse click recently-ish.
And yes, Bug 360731 doesn't really have much to do with this, as far as I see.

event.buttons should reveal whether 4th or 5th button has been pressed.
But if OS sends up back/forward commands, that has nothing to do with mouse.

Comment 6

[Olli Pettay [:smaug][[e-mail]](vacation until July 14)]

If OS sends both back/forward commands and mouse clicks for 4th/5th buttons, maybe there should be
some way to prevent the other one to happen.
Like, if mouse event happens first, calling preventDefault() could perhaps cancel
back/forward, assuming we can map the mouse input to the app command.
Is that possible using Win APIs?

Comment 7

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

I guess that it could be to dispatch mouse button event first from WM_APPCOMMAND handler. But I need to research it. (Anyway, it will be modified for similar reason for key events, see bug 865561.)

Comment 8

[Steve Collins]

First of all, I really want to thank you for taking a look at this for me.

I've been though the code pretty thoroughly and the windows widget code applies  WidgetMouseEvent::e4thButtonFlag when xbutton1 is pressed in the KeyboardLayout.cpp

it is established that MouseEvent.buttons in the DOM 3 read the forth and fifth buttons
https://developer.mozilla.org/en-US/docs/Web/API/MouseEvent.buttons

I used Spy++ (a windows visual studio support program) to watch all windows messages going to the firefox window when the 4th and 5th buttons are pressed. There are two things happening, and I'm starting to believe that we are both right and that there is an easy fix.

Here is a dump of the Spy++ capture:
<000455> 0007058C S WM_SETCURSOR hwnd:0007058C nHittest:HTCLIENT wMouseMsg:WM_XBUTTONDOWN
<000456> 0007058C R WM_SETCURSOR fHaltProcessing:False
<000457> 0007058C P WM_XBUTTONDOWN fwKeys:MK_XBUTTON1 | 00010000 xPos:394 yPos:538
<000458> 0007058C S WM_NCHITTEST xPos:488 yPos:731
<000459> 0007058C R WM_NCHITTEST nHittest:HTCLIENT
<000460> 0007058C S WM_NCHITTEST xPos:488 yPos:731
<000461> 0007058C R WM_NCHITTEST nHittest:HTCLIENT
<000462> 0007058C S WM_SETCURSOR hwnd:0007058C nHittest:HTCLIENT wMouseMsg:WM_XBUTTONUP
<000463> 0007058C R WM_SETCURSOR fHaltProcessing:False
<000464> 0007058C P WM_XBUTTONUP fwKeys:00010000 xPos:394 yPos:538
<000465> 0007058C S WM_APPCOMMAND
<000466> 0007058C R WM_APPCOMMAND
<000467> 0007058C S WM_SETTEXT lpsz:00DBDDC8 ("MouseEvent - Web API Interfaces | MDN - Mozilla...")
<000468> 0007058C S WM_STYLECHANGING wStyleType:GWL_EXSTYLE | GWL_STYLE lpss:00EBEEC8
<000469> 0007058C R WM_STYLECHANGING
<000470> 0007058C S WM_STYLECHANGED wStyleType:GWL_EXSTYLE | GWL_STYLE lpss:00EBEEC8
<000471> 0007058C R WM_STYLECHANGED
<000472> 0007058C S WM_STYLECHANGING wStyleType:GWL_EXSTYLE | GWL_STYLE lpss:00EBEEC8
<000473> 0007058C R WM_STYLECHANGING
<000474> 0007058C S WM_STYLECHANGED wStyleType:GWL_EXSTYLE | GWL_STYLE lpss:00EBEEC8
<000475> 0007058C R WM_STYLECHANGED
<000476> 0007058C R WM_SETTEXT fSucceeded:True

S/R column is for sent and received. If you look right after the WM_XBUTTONUP message there is a SENT WM_APPCOMMAND. Not shown in the log but pulled from Spy++ is the "lParam" for that app command is 80010000
ok so now it all comes together
If you look at this page:http://msdn.microsoft.com/en-us/library/windows/desktop/ms646275%28v=vs.85%29.aspx

you can see that value "1" = APPCOMMAND_BROWSER_BACKWARD

This is the windows message firefox IS processing.
if you read further down the msdn page you can see why this happens. This is an excerpt:
Remarks

DefWindowProc generates the WM_APPCOMMAND message when it processes the WM_XBUTTONUP or WM_NCXBUTTONUP message, or when the user types an application command key.

If a child window does not process this message and instead calls DefWindowProc, DefWindowProc will send the message to its parent window. If a top level window does not process this message and instead calls DefWindowProc, DefWindowProc will call a shell hook with the hook code equal to HSHELL_APPCOMMAND.


So what is happening, it that firefox is getting the WM_XBUTTONUP message and not processing it and throwing back to the DefWindowProc, which in turn sends the WM_APPCOMMAND for APPCOMMAND_BROWSER_BACKWARD.

First of all, the MSDN page says that if you process the this WM_APPCOMMAND you should return TRUE, which I'm pretty sure firefox isn't doing.

Now there are two ways to fix the mouse button thing.
1. Actually process the WM_XBUTTON messages and don't send them to DefWindowProc.
2. This is the slightly more complex but probably the easier one and it works well for having a about:config option.
Then MSDN page says, and if you look at the recorded "lParam" the first number is "8" if the command came from a mouse and "0" if it came from a keyboard.
All you have to do is when you process the WM_APPCOMMAND message, check this first number, and if it is an "8" then it came from a mouse and should be ignored if the 4th&5th mouse button option is off.

Thanks again!
Steve Collins

Comment 9

[Steve Collins]

The "it came from a mouse" this was called 
FAPPCOMMAND_MOUSE = 0x8000

Comment 10

[Ciprian Muresan [:cmuresan], Ecosystem QA]

Hi Steve, 

In my opinion this is not a browser issue and if you want to set a different command for your mouse buttons, you should use the mouse software.

Can you please try to reproduce this on the latest release(43.0.3) and latest Nightly (https://nightly.mozilla.org/) and provide the results? When doing this, please try to reproduce with a new clean Firefox profile, maybe even in safe mode, as some of this issues may be caused by third party installed add-ons or custom settings(https://support.mozilla.org/en-US/kb/troubleshoot-and-diagnose-firefox-problems).


Thanks,
Cipri

Comment 11

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

(In reply to Olli Pettay [:smaug] from comment #6)
> If OS sends both back/forward commands and mouse clicks for 4th/5th buttons,
> maybe there should be
> some way to prevent the other one to happen.
> Like, if mouse event happens first, calling preventDefault() could perhaps
> cancel
> back/forward, assuming we can map the mouse input to the app command.

Currently, looks like that we don't handle WM_XBUTTONDOWN nor WM_XBUTTONUP message. So, this must be a bug. However, I'm not sure if preceding mouse events for 4th and 5th button should be able to be prevented its default especially when the action is browser back or browser forward from a point of view of security.

I guess that the reason why we don't handle them is not allowing to prevent its default action.

> Is that possible using Win APIs?

See comment 8. If we don't send the messages to DefWindowProc, WM_APPCOMMAND shouldn't be sent.
https://msdn.microsoft.com/ja-jp/library/windows/desktop/ms646245%28v=vs.85%29.aspx (the last paragraph of "Remarks" section)


Anyway, I think that XBUTTON messages should cause mouse events but they shouldn't be able to be prevented the default action. However, this approach may cause double action which you don't like (you said in bug 1203059). From this point of view, current behavior (not sending 4th nor 5th button event) may be reasonable.

Comment 12

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

(In reply to Ciprian Muresan from comment #10)
> In my opinion this is not a browser issue and if you want to set a different
> command for your mouse buttons, you should use the mouse software.

Hmm, I guess that this is available only when the user has multifunctional mouse (typically expensive). IIRC, even without mouse utils, 4th button causes browser back with WM_APPCOMMAND and 5th button causes browser forward with WM_APPCOMMAND. In fact, when I disable 4th button or 5th button with mouse utility, XBUTTON messages won't be sent.

Comment 13

[Olli Pettay [:smaug][[e-mail]](vacation until July 14)]

Not sure what I'm being asked here :)

Our current behavior of not sending mouse events for those XBUTTON seems rather ok, especially if we can't prevent the possible following appcommand.
But perhaps there could be a pref to disable appcommand handling altogether?
Or the behavior of http://mxr.mozilla.org/mozilla-central/source/browser/base/content/browser.js#1604 
could rely on some prefs? A question is that would more than just couple of people use the prefs.
Adding more and more prefs makes code harder to maintain.

Comment 14

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

(In reply to Olli Pettay [:smaug] from comment #13)
> Not sure what I'm being asked here :)

Sorry for the not clear comment. I should've just added "How do you think?".

> Our current behavior of not sending mouse events for those XBUTTON seems
> rather ok, especially if we can't prevent the possible following appcommand.

Okay.

> But perhaps there could be a pref to disable appcommand handling altogether?
> Or the behavior of
> http://mxr.mozilla.org/mozilla-central/source/browser/base/content/browser.
> js#1604 
> could rely on some prefs? A question is that would more than just couple of
> people use the prefs.

I think that there should be a pref in windows widget level because it's bad design users cannot disable 4th nor 5th button cannot disable with system settings of Windows (it may be possible if the pointing device have its own utility). So, adding a pref may help users using cheaper mouse and not hoping to use these buttons.

> Adding more and more prefs makes code harder to maintain.

I agree, but I don't think that we should worry about that in this case because we should fix this with very simple patch.

However, I have a question. If we disable dispatching APPCOMAND events to chrome, should we dispatch normal mouse button events for 4th and 5th buttons? Or shouldn't do that for consistency?

Comment 15

[Olli Pettay [:smaug][[e-mail]](vacation until July 14)]

Well, which web page would use those events for anything if effectively no one has system which dispatches them by default?

Perhaps some games would like to be able to use extra buttons, but does any other browser dispatch
4th or 5th button down/up.

So, I'd say no need to dispatch mouse events here, unless there is some clear reason.

Comment 17

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=5df5921c9b65

Comment 18

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Attachment: Allow users to disable 4th button and/or 5th button of mouse on Windows

Okay, let's just create new prefs to allow users to disable mouse buttons since Windows cannot have a way in its control panel (if the mouse's utility doesn't provide it).

Comment 19

[Olli Pettay [:smaug][[e-mail]](vacation until July 14)]

Comment on attachment 8708276 [details] [diff] [review]
Allow users to disable 4th button and/or 5th button of mouse on Windows

jimm needs to review that WM_* and XBUTTON* usage is right.
msdn is very vague what WM_*XBUTTON* means.

aRetValue is not a bool, so better to assign TRUE or FALSE to it (for consistency)?

Comment 20

[Jim Mathies [:jimm]]

Comment on attachment 8708276 [details] [diff] [review]
Allow users to disable 4th button and/or 5th button of mouse on Windows

Review of attachment 8708276 [details] [diff] [review]:
-----------------------------------------------------------------

::: widget/windows/nsWindow.cpp
@@ +5413,5 @@
>                                    MOUSE_INPUT_SOURCE());
>        DispatchPendingEvents();
>        break;
>  
> +    case WM_XBUTTONDOWN:

can we add a little block comment here explaing what this code does? similar to your comment in the prefs file. Thanks!

Comment 21

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/9fd8d0a02f2ba216493dcda33db572a2f4df5db3
Bug 1052947 Allow users to disable 4th button and/or 5th button of mouse on Windows r=smaug+jimm

Comment 22

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/9fd8d0a02f2b