[Tony Brix (UziTech)]

Add middle mouse button scrolling

fixes #6302

[Tony Brix (UziTech)]

@microsoft-github-policy-service agree

[Tony Brix (UziTech)]

I can create more tests but I wanted to get an idea if this was the right way to add middle button scrolling first. I didn't get any response on #6302 (comment)

[Jonathan (Paril)]

This is literally life-saving and I would pay $5 a month for this feature.

[Glandos (Glandos)]

Silly question: does it block text paste on X11 with middle click?
ScrollAnywhere for Firefox has this feature:

Linux users may want to enable "Don't block default button action" option in Options/Advanced - to enable 'Paste' with middle button again.

However, the extension is closed source.

[Tony Brix (UziTech)]

Silly question: does it block text paste on X11 with middle click?

The setting is off by default so it should not prevent paste on middle click but when turned on it will prevent it.

This does have two separate modes currently. If the user holds down the middle button and moves the cursor around it will cancel the scrolling on mouse up, and if the user clicks the middle button the scrolling will continue until the middle button is clicked again.

Maybe this could be two settings so holding the button will scroll and clicking the button will paste?

[Rasit-Vatan]

A beautiful attemp after 8 years of waiting!!! HYPPE!

[Glandos (Glandos)]

This does have two separate modes currently. If the user holds down the middle button and moves the cursor around it will cancel the scrolling on mouse up, and if the user clicks the middle button the scrolling will continue until the middle button is clicked again.

Maybe this could be two settings so holding the button will scroll and clicking the button will paste?

If the 2 behavior can be implemented, that would be perfect.

But for Linux users, it would be a really pain that middle click doesn't paste, so I would much prefer the second solution (scroll on hold)

[Henning Dieterichs (hediet)]

Thanks a lot for this PR!
Can you look into the CI failure?

I will be out for the next 2 weeks, so we might not be able to take it before the May release.

[Tony Brix (UziTech)]

It looks like the tests are failing because editor.getDomNode() is undefined in the test environment even though it should never be undefined according to https://github.com/UziTech/vscode/blob/389cc256296c20f1782e719ecb646f30c43b3f51/src/vs/editor/browser/editorBrowser.ts#L1242

Is there something different I need to do in the tests to get editor.getDomNode()?

[Tony Brix (UziTech)]

Glandos (@Glandos) could you test this with linux and paste on middle click? I don't have a linux setup to test. Is paste on middle click on mousedown or mouseclick?

[Glandos (Glandos)]

Glandos (@Glandos) could you test this with linux and paste on middle click? I don't have a linux setup to test. Is paste on middle click on mousedown or mouseclick?

I'm sorry to ask a silly question: how do you test this? Is there a CI job that build a version for each PR?

[Tony Brix (UziTech)]

Glandos (@Glandos) I don't think there is a binary built for each CI run.

Henning Dieterichs (@hediet) is there an easy way to test this on Linux?

Otherwise this is at least off by default so it won't change the behavior. If there is a bug on Linux we can fix it after release.

[MarcoFilimon]

We're still waiting for this amazing feature, 8y later!

[Tony Brix (UziTech)]

I was able to setup a linux vm and test this with paste on middle click. What I have found is that scroll on middle button and paste on middle click cannot be used together because scroll on middle button requires a mouse down events. Knowing before the mouse down event which one the user intends is not likely possible.

Possible solutions:

• Alert the user if both are turned on that they will conflict if both are turned on in the settings.
• When both are enabled in the settings have a dialog pop up asking which one the user wants to enable since they can't have both.
• Share a setting that is a dropdown so the user can only choose one or none.

Henning Dieterichs (@hediet) thoughts?

[Tony Brix (UziTech)]

The current situation is that we have 3 settings that can determine what happens on middle button click:

• editor.selectionClipboard
• editor.columSelection
• editor.scrollOnMiddleClick

I think it would be good to change it to one setting that is a dropdown but that can be done after this PR

[Henning Dieterichs (hediet)]

Thanks for testing on linux!
Had a closer look, found some more things:

• When the editor is moved into a new window, the mouse listeners don't work anymore (is there a way to avoid the global listeners? Alternatively, we have to watch window reparenting somehow).
• Not sure if addEventListener and removeEventListener with different options will leak the listeners (on the global object!). addEventListener/remove... is always a footgun, that's why we have addDisposableListener.

I refactored the code change in the mouse dispatching logic a bit (shouldn't change anything though), to go from one special case with a special case exclusion to two special cases. I'm not very familiar with the middle mouse button pasting mechanism on linux, but if this only gets a problem if the user turns on two middle mouse button features, it is okay that they conflict (and one of them does not work).

I was on vacation again and I think this needs a little more polishing and testing, so let's try to merge it early next iteration (i.e. next week)!

[Tony Brix (UziTech)]

is there a way to avoid the global listeners?

We can put the listeners on whatever element we want but the user will have to mouse up/down/move on that element in order to affect the scrolling.

In other words, if we put the listeners only on the editor and someone clicks the middle button to start scrolling they can only stop scrolling by clicking the editor again. If they click or mouse up outside of the editor it won't stop scrolling.

[Tony Brix (UziTech)]

Henning Dieterichs (@hediet) anything else we need to do to merge this?

[Henning Dieterichs (hediet)]

I noticed some more problematic things, so I introduced observables in your implementation. I think we can merge it now!
This should fix:

• leaks, as any disposable added through this._register accumulate in this
• no mouse event listening when the feature is disabled
• consistent scrolling speeds, even when the user has higher or lower fps
• problems when the text editor switches the model and the editor dom node is recreated

However, I didn't figure out a quick way to write tests for this new implementation.
To be honest though, I doubt this implementation has to be touched again.

Thanks for the PR again!

Are you up to write release notes for your feature and provide some testing steps or test the feature yourself? :)

[Tony Brix (UziTech)]

One thing I noticed in testing is that your changes removed the scroll on click and only left the scroll on holding the middle mouse button down. I think that is a good start, but I know from adding this to Atom that a lot of people prefer clicking the middle button then moving the mouse then clicking again to end scrolling.

[Henning Dieterichs (hediet)]

I think that is a good start, but I know from adding this to Atom that a lot of people prefer clicking the middle button then moving the mouse then clicking again to end scrolling.

Good point, missed that! Fixed in #251393.

[Axel D. (SetTrend)]

Henning Dieterichs (@hediet):

Can we have this scroll-lock behaviour optional, please?

Scroll lock is particularly annoying for left-handed people, like me, who accidentally hit the middle mouse button frequently.

For reference, see this Bugzilla issue: https://bugzilla.mozilla.org/show_bug.cgi?id=1923233

[Henning Dieterichs (hediet)]

Can we have this scroll-lock behaviour optional, please?

If there is enough demand, I'm up to add a setting for this (default with scroll lock though, as at least chrome also works like that).
Can you create a new feature request?

[Axel D. (SetTrend)]

Thanks for considering my concerns. Feature request created: #251609