Fix issue #67: Double right-click to open context menu in OSX/Linux

[pmorch]

This is for issue #67

[marklieberman]

Is there a particular reason you chose to use setTimeout instead of just measuring the difference in milliseconds between first and second click?

[pmorch]

:-) No, not really... This was the solution that was used by the the other implementations: Mouse Gesture Events and the couple of Chrome extensions I looked at. Do you want me to change it?

Any other, more stylistic comments? I tried to stay as close to your coding style as I could.

[marklieberman]

Its pretty good, the only stylistic issue is that I generally use the arrow notation for small callbacks. So L191 up there would be: setTimeout(() => { ... }).

I'll merge the PR soon; I just moved and haven't prepared a good workspace to do anything besides check emails.

[marklieberman]

Would you mind wrapping this functionality behind a preference? Based on recent comments in https://bugzilla.mozilla.org/show_bug.cgi?id=1360278 someone has provided a build that implements contextmenu on mouse up. If that's the case, I'd like for testers to be able to switch double right-click off without having to do a release specifically for that. You could just add a checkbox to general tab.

[pmorch]

I agree - I just tested the beta posted in https://bugzilla.mozilla.org/show_bug.cgi?id=1360278 and it works. Lets hold off merging this and see if that fix makes it into the final Firefox 57. If it does, then we'll close this and forget about the whole thing.

[pmorch]

Oh, my bad. It was an external user that posted the beta. The same user that submitted the patch. I thought it was a Firefox team member.... Lets wait a while and see whether the there is progress on the issue. I just got a bubble of hope that now has burst somewhat.

If it comes to it, sure I'll put the functionality behind an option if you like. Next question is then what the default value should be. My guess is that the patch doesn't make it into Firefox 57, it'll probably take ages. So I suggest that the option should be hidden for non OSX/Linux installations, and default to double-right-click until the patch is accepted, so that it at least works.

The ideal solution would be feature-detection. You probably know more about this than I do. Is there some way to detect if the context menu is shown on mouse down or mouse up? If the context menu is shown on mouse down, then use the double-right-click approach. No platform/userAgent sniffing, no option, and the ugly double-right-click business will stop transparently and automatically if/when the patch gets applied.

[marklieberman]

I don't think this is something that can be feature detected. Also, as it turns out instead of doing:

const isLinuxOSX = window.navigator.userAgent.indexOf("Linux") !== -1 ||
                   window.navigator.userAgent.indexOf("Macintosh") !== -1;

You should be using the runtime.getPlatformInfo() API to get the platform enum value. I just noticed this while scanning the API docs.

I think you should go ahead with the changes we've discussed. Default the double right-click setting to enabled. I am certain the patch will not be merged. It will be 58 or 59 at the earliest. Nobody has tested it on OSX. It won't be merged until its behind a about:config pref and its consistent on Linux and OSX.

Once we know what version will be released with the Bugzilla bug patched, I can add a handler to runtime.onInstalledReason for browser_update to disable it for affected users.

[gwarser]

Any chance this can be made ctrl+right click or rocker gesture (left -> right)?
Using smartUp Gestures with double click for some time, and now I double-right-click everywhere :)

[imrn]

+1 for (left -> right) rocker.

[marklieberman]

@gwarser This PR is about implementing the same behaviour as smartUp on OSX/Linux.

[gwarser]

@marklieberman yes, but now I have muscle memory for double right click, and will like to change this, because this is annoying in other applications.

[pmorch]

About this checkbox on the general tab: How about browser.storage.sync vs browser.storage.local. If several browsers all sync, I think it makes no sense for this setting also to get synced. The reason is that not all browsers run on OSX/Linux and not all of them will have right-click menu fixed. So do you agree to put this one setting in browser.storage.local?

[pmorch]

About the (left -> right) rocker request: Do we agree, that if we go this route, the default action for the first button in rocker - the left click - will be fired when the left click occurs? If so, I don't think it will be e.g. possible to select something and then (left -> right) rocker, and then hit "Copy", because the first left-click of the rocker will deselect the selection.

So I don't know how this could work...

[marklieberman]

I am currently working on chord gesture support and its 99% finished. Having just dealt with all the issues around handling multiple button presses in the context of gestures, I do not wish to support (left->right) as a gesture trigger for non-chord gestures.

I agree that the double right-click setting should live under storage.local.

[pmorch]

Hi, just an update.

Yes, I know. Release date for Firefox Quantum is tomorrow. Private life has interfered, and I don't yet have a pull request ready for this. Damn. :-( But I haven't given up yet.

Having some settings in storage.local and some in storage.sync will make this pull request a little larger than I originally anticipated, I'm afraid. Just so there are no surprises later. This is the stuff that is missing.

I'll post here as soon as I have something (anything) to show.

Peter

[pmorch]

Please see #109 as an alternative pull-request instead.