search

qsniyg / maxurl

Finds larger/original versions of images and videos
https://qsniyg.github.io/maxurl/
Apache License 2.0
1.11k stars 69 forks source link

Disabling right click menu when m2 is a shortcut #680

Open samwolfe2000 opened 3 years ago

samwolfe2000 commented 3 years ago

Hey!

I have a problem that (with Firefox) whenever I use mouse2 as a shortcut the right click menu pops up. This makes it quite unconfortable to use as always having to close that square afterwards.

Right click btw would be an important shortcut element for those who don't want to use the keyboard for shortcuts.

Thanks in advance!

qsniyg commented 3 years ago

Thanks for reporting, I might have fixed it with https://github.com/qsniyg/maxurl/commit/f61386b21447af1c06664ea5b7272ff2d6974626, can you test?

qsniyg commented 3 years ago

After further testing, it appears to be fixed, so I'll close the issue. If it isn't however, please let me know! :)

samwolfe2000 commented 3 years ago

Hey!

Sorry for the late reply, I didn't notice your comment.

Thank you for your efforts, however, I'm not sure how to try this version out. As far as I can see it hasn't been officially released and I can't find a way to install it manually. Isn't there supposed to be an .xpi file somewhere?

Thanks again.

qsniyg commented 3 years ago

Sorry, in order to test the firefox version, you have to load it via about:debugging -> Load temporary addon, then select "manifest.json" from the repository directory. Alternatively, you can just install the userscript using violentmonkey (or tampemonkey) to test, there shouldn't be any differences between the two for this.

As far as I can tell right clicking seems to work properly with the above change, but it's possible that there might be some sites where it doesn't work properly, overriding it can be a bit tricky.

samwolfe2000 commented 3 years ago

Well, I tried it out and strange things happened. Our original problem has not been solved, but a new one has been generated: now two popups can be opened at the same time.

I made s short showcase of the phenomenon: https://www.youtube.com/watch?v=OlKTE7ymlTc

I have the following remarks:

I attach my IMU settengs: Image Max URL setup_samwolfe.txt

Have you any idea what can cause this issue?

Greetings,

samwolfe

qsniyg commented 3 years ago

Thanks for the settings, I can replicate the right click issue with them. The issue is that the script currently only prevents the right click menu at the end of a key chord, but for button3+button2+wheelup, button2 is likely not going to be the last one. As long as button2 is not at the start of the sequence, I should be able to make the script support it (I'll just need to rework a bit of logic first).

If you're wondering, the reason that button2 cannot be supported at the start of a sequence is that it's impossible for the script to know whether the user wants to open the right click menu, or start a key sequence (since the right click menu is opened on button press, not on button release).

For the two popups issue, for some reason I'm unable to replicate this. I've tried to follow along what you did in the video, but I'm not sure how you attained it (pressing S beside it will always close the popup if it's open for me). I'm sorry to ask for this, as I know it's a bit more work, but would you mind creating a step-by-step guide to reproduce it? In the meantime, I'll try to see if I can find any leaks in the code that could have caused this.

To be clear, the script only supports one popup at a time, so if for any reason there are two popups open, any behavior thereof is very likely going to be incorrect. Supporting multiple popups is going to take a lot of reworking unfortunately (as I had the rather stupid idea of storing everything related to the currently opened popup in many global variables).

samwolfe2000 commented 3 years ago

(since the right click menu is opened on button press, not on button release)

I'm not sure I understand what you're saying here. The right click menu appeares after release, and there are add-ons utilizing this "feature". The simplest example is Scroll Zoom which prevents the menu from appearing when the mouse wheel is triggered. Needless to say that such a behavior would be convenient in IMU as well.

I noticed the dubble popup behavior by having the popup trigger key set to mouse3 as well. In this case whenever I tried to use mouse3 + mousewheel for zooming, it opened a new popup. I can't say the behavior occures 100% of the time, as there are cases when it does not. But when it decides to occure, its operation is systematic.

Maybe try this over here

At the moment I opened the linked page for a second time, and only the one opened first produces this behavior.

qsniyg commented 3 years ago

The right click menu appeares after release, and there are add-ons utilizing this "feature".

Perhaps this is a difference between windows and linux then, I'll try this under a VM.

But when it decides to occure, its operation is systematic.

I'll try to look more carefully through the code to see if there's anything that could cause this, because I'm currently entirely unable to replicate this, even with mouse3+wheel. Maybe it'll be present under the VM though... (hopefully!)

samwolfe2000 commented 3 years ago

Now it seems to work quite reliably, thank you!

Maybe an additional suggestion: couldn't you give an option to overwrite hotkeys of other addons in case of conflict? When I'm using this add-on I usually don't want to use other add-ons. For example, I don't use Scrollzoom to zoom on the page, but instead could use those hotkeys to zoom on the picture inside of IMU. This may could allow some keycombinations to be more convenient inside the add-on. I don't know if it's possible or even worth the effort.

But thanks again for the main feature, I highly appreciate your selfishless efforts.

Best regards,

Sámuel

qsniyg commented 3 years ago

couldn't you give an option to overwrite hotkeys of other addons in case of conflict?

The only way I could see this being possible is perhaps if this addon were to integrate with the webextension shortcuts api (https://github.com/qsniyg/maxurl/issues/745), but this would also require the other addons to use it too (scrollzoom for example doesn't afaics). This of course depends on whether browsers will let you know when you use the same keybinding as another addon.

I haven't used the API at all so I don't know what the limitations are. AFAICS though it doesn't support any kind of mouse-related shortcuts (buttons, wheel movement), so I don't know how easily integrate-able it would be (e.g. when setting it to mouse-related shortcuts, it would need to unset the webextension command).

That said, when it overrides a key, it should usually prevent other handlers from running if it's handling the key. Of course, this depends on when it's being loaded onto the page. If it's loaded after another addon that also uses the same listeners, then it will likely fail. There's not much I can do about that afaik unfortunately, no good solution has yet been found for https://github.com/qsniyg/maxurl/issues/38.