Added mouse wheel scrolling commands by cary-rowen · Pull Request #16462 · nvaccess/nvda

cary-rowen · 2024-04-30T03:14:22Z

Link to issue number:

Summary of the issue:

Some dynamically loaded web pages and desktop applications, such as Dism++, only allow scrolling via the hardware mouse wheel to display more content. NVDA can only interact with these through screen review mode. However, to load more items, mouse wheel usage is required.

Description of user facing changes

This pull request allows users to simulate mouse wheel scrolling through gestures. Added four commands with no gestures assigned:

Scroll up at the mouse position
Scroll down at the mouse position
Scroll left at the mouse position
Scroll right at the mouse position

Description of development approach

I created a function within mouseHandler called scrollMouseWheel, which scrolls the mouse wheel either vertically or horizontally, controlling the scroll direction and amount. For more information about mouse events, see WM_MOUSEWHEEL.
It's important to note that this function limits the scrolling per execution to winUser.WHEEL_DELTA (120) or less. Although this PR performs the standard scrolling amount of 120 each time, add-on developers might opt for a larger amount, as done by @beqabeqa473 in mouseWheelScrolling.

Testing strategy:

I tested the following cases:

Go to System Settings > Devices > Mouse > Roll the mouse wheel to scroll:

Multiple lines at a time
One screen at a time

NVDA+Ctrl+V opens the voice settings panel and simulates mouse wheel scrolling on slider controls such as speed and volume.
Try simulating mouse wheel scrolling in Microsoft Excel and visually verify that the scrolling is as expected.

Known issues with pull request:

Microsoft Excel correctly responds to system settings, whether "Multiple lines at a time" or "One screen at a time".

However, the slider in the NVDA voice settings panel always scrolls at the standard amount of 120 and does not respond to system settings, but is consistent with the behavior of physical mouse wheel.

Code Review Checklist:

Documentation:
- Change log entry
- User Documentation
- Developer / Technical Documentation
- Context sensitive help for GUI changes
Testing:
- Unit tests
- System (end to end) tests
- Manual testing
UX of all users considered:
- Speech
- Braille
- Low Vision
- Different web browsers
- Localization in other languages / culture than English
API is compatible with existing add-ons.
Security precautions taken.

cary-rowen · 2024-04-30T03:26:16Z

Here are some things I'm not sure about yet:

Due to PR merge freeze until Monday May 6 AEDT #16419 I don't know where to put the change log entry.
Is it necessary for scrolling gestures to play a beep or ui message when scrolling is performed to inform the user that the operation has been performed?

AppVeyorBot · 2024-04-30T03:51:18Z

PASS: Translation comments check.
PASS: Unit tests.
FAIL: Lint check. See test results for more information.
PASS: System tests (tags: installer NVDA).
Build (for testing PR): https://ci.appveyor.com/api/buildjobs/4dxjbtw1b1jr5bx4/artifacts/output/nvda_snapshot_pr16462-31682,ac5a0ac7.exe
CI timing (mins):
INIT 0.0,
INSTALL_START 1.1,
INSTALL_END 1.1,
BUILD_START 0.0,
BUILD_END 30.2,
TESTSETUP_START 0.0,
TESTSETUP_END 0.4,
TEST_START 0.0,
TEST_END 2.5,
FINISH_END 0.2

See test results for failed build of commit ac5a0ac7a4

Adriani90 · 2024-04-30T04:16:09Z

@cary-rowen nice feature, thanks very much for this great contribution.
You write:

Due to PR merge freeze until Monday May 6 AEDT PR merge freeze until Monday May 6 AEDT #16419 I don't know where to put the change log entry.

I think 2024.2 will not take additional change log entris from alpha. But better we wait for NV Access confirmation on that.

Is it necessary for scrolling gestures to play a beep or ui message when scrolling is performed to inform the user that the operation has been performed?

NVDA should read out what is under the mouse cursor, as it does when moving the mouse.
Beeps are not an option in my opinion, because you don't have any clue where the mouse scrolled if NVDA doesn't read anything out.

Also we need to test if the review cursor is moved properly when it follows the mouse. You can enable this in the review cursor settings. Did you test this already?

CyrilleB79 · 2024-04-30T06:49:33Z

Re the change log:
You will have to put it in the correct section (2024.3) when it is created in alpha.

hwf1324 · 2024-04-30T07:19:20Z

NVDA should read out what is under the mouse cursor, as it does when moving the mouse. Beeps are not an option in my opinion, because you don't have any clue where the mouse scrolled if NVDA doesn't read anything out.

Note that this is not the same behavior as actually scrolling the physical scroll wheel.
It does not actually report new content under the mouse when the screen is scrolled.

LeonarddeR

As for the gestures, I would personally vote against gestures that do not involve the NVDA key. I'm also not convinced that these gestures need assignments by default. Both UIA and IAccessible2 have the ability to instruct objects to scroll into view.

That said, I think this is certainly a valuable feature!

source/mouseHandler.py

XLTechie · 2024-05-01T03:38:24Z

Happy to see this feature! I also would prefer that NVDA core avoid gestures that do not include the NVDA key. For me, assigning these gestures are fine, if they include NVDA key also.

cary-rowen · 2024-05-02T06:14:45Z

Thanks to @LeonarddeR for the code review.

@LeonarddeR and @XLTechie

I assigned it a gesture with NVDA keys, NVDA+Windows+pageUp/Down hoping this would be more useful, I'd also like to hear what the community has to say.

Hi @Adriani90

NVDA should read out what is under the mouse cursor, as it does when moving the mouse.

This PR is intended to emulate the scroll wheel functionality on a physical mouse, which does not behave exactly like a mouse pointer.

@CyrilleB79

You will have to put it in the correct section (2024.3) when it is created in alpha.

No problem, I will do it.

CyrilleB79 · 2024-05-02T09:16:17Z

The need to scroll seems quite rare; personally, I would not have assigned it any keyboard shortcut.

cary-rowen · 2024-05-05T01:04:24Z

Scrolling the wheel is probably rarer than moving the mouse pointer, I'd be willing to have the default gesture removed.

AppVeyorBot · 2024-05-05T01:46:29Z

PASS: Translation comments check.
PASS: Unit tests.
PASS: Lint check.
PASS: System tests (tags: installer NVDA).
Build (for testing PR): https://ci.appveyor.com/api/buildjobs/n4pifls3r6ng06yi/artifacts/output/nvda_snapshot_pr16462-31725,20ccf788.exe
CI timing (mins):
INIT 0.0,
INSTALL_START 1.0,
INSTALL_END 1.0,
BUILD_START 0.0,
BUILD_END 28.0,
TESTSETUP_START 0.0,
TESTSETUP_END 0.4,
TEST_START 0.0,
TEST_END 2.4,
FINISH_END 0.5

See test results for failed build of commit 20ccf7889e

seanbudd · 2024-05-06T06:19:36Z

source/globalCommands.py

+		category=SCRCAT_MOUSE
+	)
+	def script_scrollMouseWheelDown(self, gesture):
+		mouseHandler.scrollMouseWheel(-winUser.WHEEL_DELTA, True)


Suggested change

mouseHandler.scrollMouseWheel(-winUser.WHEEL_DELTA, True)

mouseHandler.scrollMouseWheel(-winUser.WHEEL_DELTA, isVertical=True)

source/globalCommands.py

source/mouseHandler.py

seanbudd · 2024-05-06T06:27:13Z

Can you please update the description to reflect the latest changes e.g. note that the gestures are unbound

CyrilleB79 · 2024-05-06T07:43:56Z

source/globalCommands.py

+	@script(
+		description=_(
+			# Translators: Input help mode message for mouse wheel scroll up command.
+			"Scrolls up the mouse wheel at the current mouse position"


Technically, I think that you do not scroll the mouse but the window's content at the mouse position, e.g. you scroll a webpage.
English native speakers, please confirm.

Suggested change

"Scrolls up the mouse wheel at the current mouse position"

"Scrolls up at the current mouse position"

Yes, most of the time it's not the mouse wheel itself that's scrolling, but rather the content on the screen (such as a web page or document) that moves in response to the mouse wheel's operation.
However, from a technical implementation perspective, this is the result of simulating the behavior of a mouse wheel scrolling.
The function doesn't move the content directly; it generates mouse wheel events that the system interprets as commands to scroll the content.
Can other native English speakers offer more advice on this?

Yes I think this terminology is better for these descriptions

Do you mean I don't need to make changes or should I take @CyrilleB79's suggestion?

@cary-rowen, if you are concerned by the fact that the function does not move the content directly, do not use scroll at all, because the function does not actually perform scrolling, it actually simulate wheel rotating.

Suggested change

"Scrolls up the mouse wheel at the current mouse position"

"Rotates up the mouse wheel at the current mouse position"

IMO, no need to mention "simulation", as done also for left/right mouse click. Indeed, assigning a keyboard or touch gestures already makes clear that the real action is not done by the physical mouse.

@Qchristensen - what are your thought on the changes from scrolling to rotating? I prefer scrolling and think its more common. Rotating to me seems to imply physical movement of the wheel, not the behaviour on screen.

I agree that the verb "to scroll" is more common and probably more understandable.

Actually, my concern with the initial wording was that "to scroll up the mouse wheel" was not correct grammatically. Why not things like:

"scroll up at the mouse position" for descriptions / comments

"mouseScrollUp" for the script's name?

No matter if we do not mention the word "wheel", which create confusion or bad phrasing when used with "scroll".

No matter if we do not mention the word "wheel", which create confusion or bad phrasing when used with "scroll".

I don't entirely agree with this.

I have attached a Microsoft document before. Can we refer to some of the wording in it?

https://support.microsoft.com/en-us/topic/what-are-the-default-buttons-and-wheel-assignments-for-my-mouse-47f656a8-72d0-63ec-8d1d-dda7122bef6b

But "scrolling" is actually a key word present in the documentation you linked (first table, in the column "Action")

IMO, when possible, it's better to mention an action rather than a physical button, because there may be other ways to perform them.
For example:

use "scroll up/down" rather than "rotate the mouse wheel up/down" because there may be other way to perform these actions, e.g. with a touch pad.

use "scroll left/right" rather than "rotate the mouse wheel left/right because in Chrome you can scroll horizontally using shift + rotating vertically the mouse wheel.

CyrilleB79 · 2024-05-06T07:47:31Z

source/globalCommands.py

+		),
+		category=SCRCAT_MOUSE
+	)
+	def script_scrollMouseWheelUp(self, gesture):


Idem:

Suggested change

def script_scrollMouseWheelUp(self, gesture):

def script_scrollUpAtMousePosition(self, gesture: inputCore.InputGesture) -> None:

user_docs/en/userGuide.t2t

user_docs/en/changes.md

source/globalCommands.py

XLTechie · 2024-05-14T05:16:34Z

source/mouseHandler.py

@@ -379,3 +379,32 @@ def unlockRightMouseButton():
 	# Translators: This is presented when the right mouse button lock is released (used for drag and drop).
 	ui.message(_("Right mouse button unlock"))


I know this is old, but...

Suggested change

ui.message(_("Right mouse button unlock"))

ui.message(_("Right mouse button unlocked"))

I would like to do the left one as well, but when expanding up to it, GH won't allow a change suggestion.

cary-rowen · 2024-05-14T16:32:05Z

Thanks to @XLTechie for the English wording suggestion.

cary-rowen · 2024-05-16T13:23:14Z

@seanbudd I think this is ready.

Adriani90 · 2024-05-17T21:33:43Z

@cary-rowen has been there any testing with the visual highlighter on? What happens when the highlighters are enabled? Do they disappear from the screen when scrolling the wheel with the keyboard commands? Or do they get a new position?

hwf1324 · 2024-05-17T21:51:11Z

@cary-rowen has been there any testing with the visual highlighter on? What happens when the highlighters are enabled? Do they disappear from the screen when scrolling the wheel with the keyboard commands? Or do they get a new position?

I'm not testing it right now.
But visual highlighter not following the screen scrolling and moving in many cases is something that has been an issue before.
Update: For visual highlighter, the behavior of the physical mouse wheel and the simulated mouse wheel is the same, and neither of the highlighter indicators will change with it.

cary-rowen · 2024-05-21T00:09:10Z

cc @Qchristensen
Can you give any further actionable suggestions?

Qchristensen · 2024-05-22T04:22:49Z

Just looking at the User Guide change rather than the code comment, I see line 840 has:

|Rotates the mouse wheel up at the current mouse position|

I would go with "scroll", and yes I agree with "up" where it is in this sentence rather than where it was earlier in the conversation. While the previous suggestion of "Scroll up at the current mouse position" is shorter, I'm not against "Scroll the mouse wheel up" as it does indicate what is trying to be scrolled at this point, as opposed to the several other instances of scrolling commands in NVDA, eg:

Scrolling in the Windows Console: https://www.nvaccess.org/files/nvda/documentation/userGuide.html#WinConsole
Scrolling on a Braille display, eg: https://www.nvaccess.org/files/nvda/documentation/userGuide.html#FreedomScientificFocus

cary-rowen · 2024-06-02T03:24:48Z

@Qchristensen
Can you review these changes to the doc?

cary-rowen · 2024-06-05T12:40:59Z

Hi cc @seanbudd
I think this is ready.

source/globalCommands.py

user_docs/en/changes.md

Qchristensen

Looks good, thanks Cary and everyone!

Add simulated mouse wheel feature.

13e40af

cary-rowen requested review from a team as code owners April 30, 2024 03:14

cary-rowen requested review from Qchristensen and michaelDCurran April 30, 2024 03:14

Fix lint

25f5abd

LeonarddeR suggested changes Apr 30, 2024

View reviewed changes

LeonarddeR reviewed Apr 30, 2024

View reviewed changes

source/mouseHandler.py Outdated Show resolved Hide resolved

LeonarddeR reviewed Apr 30, 2024

View reviewed changes

source/mouseHandler.py Outdated Show resolved Hide resolved

seanbudd added this to the 2024.3 milestone May 1, 2024

cary-rowen added 3 commits May 2, 2024 13:42

Apply suggestions from code review.

65a1319

Add NVDA key to gestures

4c3c78f

Fix lint.

c506f29

No default gesture assigned

fd7fef7

seanbudd reviewed May 6, 2024

View reviewed changes

seanbudd marked this pull request as draft May 6, 2024 06:26

CyrilleB79 reviewed May 6, 2024

View reviewed changes

seanbudd added the conceptApproved Similar 'triaged' for issues, PR accepted in theory, implementation needs review. label May 7, 2024

Handle userGuide merge conflicts, And markdown format.

699eb80

XLTechie reviewed May 14, 2024

View reviewed changes

user_docs/en/changes.md Outdated Show resolved Hide resolved

XLTechie suggested changes May 14, 2024

View reviewed changes

source/globalCommands.py Outdated Show resolved Hide resolved

source/globalCommands.py Outdated Show resolved Hide resolved

source/globalCommands.py Outdated Show resolved Hide resolved

source/globalCommands.py Outdated Show resolved Hide resolved

XLTechie reviewed May 14, 2024

View reviewed changes

Apply suggestions from code review

a1f7b63

cary-rowen requested a review from seanbudd May 15, 2024 07:35

cary-rowen changed the title ~~Add simulated mouse wheel feature~~ Added mouse wheel rotation commands May 16, 2024

Merge branch 'master' into simulateMouseWheel

985a04f

seanbudd marked this pull request as draft May 23, 2024 03:24

seanbudd removed this from the 2024.3 milestone May 24, 2024

cary-rowen added 2 commits June 2, 2024 08:49

update

7aadd63

Use 'scroll' instead of 'rotate'

eaded9a

cary-rowen changed the title ~~Added mouse wheel rotation commands~~ Added mouse wheel scrolling commands Jun 2, 2024

cary-rowen marked this pull request as ready for review June 2, 2024 03:23

seanbudd reviewed Jun 6, 2024

View reviewed changes

seanbudd and others added 4 commits June 6, 2024 16:24

Apply suggestions from code review

fd3b437

Apply suggestions from code review

8424327

Merge branch 'master' into simulateMouseWheel

4b8cec3

Fix the confusion caused by merge conflicts in Changes

620f1fa

seanbudd approved these changes Jun 7, 2024

View reviewed changes

Qchristensen approved these changes Jun 7, 2024

View reviewed changes

seanbudd merged commit 2b029d1 into nvaccess:master Jun 7, 2024

cary-rowen mentioned this pull request Jun 9, 2024

Remove mouse wheel scrolling functionality mltony/nvda-tonys-enhancements#28

Merged

	mouseHandler.scrollMouseWheel(-winUser.WHEEL_DELTA, True)
	mouseHandler.scrollMouseWheel(-winUser.WHEEL_DELTA, isVertical=True)

	"Scrolls up the mouse wheel at the current mouse position"
	"Scrolls up at the current mouse position"

	"Scrolls up the mouse wheel at the current mouse position"
	"Rotates up the mouse wheel at the current mouse position"

	def script_scrollMouseWheelUp(self, gesture):
	def script_scrollUpAtMousePosition(self, gesture: inputCore.InputGesture) -> None:

		@@ -379,3 +379,32 @@ def unlockRightMouseButton():
		# Translators: This is presented when the right mouse button lock is released (used for drag and drop).
		ui.message(_("Right mouse button unlock"))

	ui.message(_("Right mouse button unlock"))
	ui.message(_("Right mouse button unlocked"))

Uh oh!

Conversation

cary-rowen commented Apr 30, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Link to issue number:

Summary of the issue:

Description of user facing changes

Description of development approach

Testing strategy:

Known issues with pull request:

Code Review Checklist:

Uh oh!

cary-rowen commented Apr 30, 2024

Uh oh!

AppVeyorBot commented Apr 30, 2024

Uh oh!

Adriani90 commented Apr 30, 2024

Uh oh!

CyrilleB79 commented Apr 30, 2024

Uh oh!

hwf1324 commented Apr 30, 2024

Uh oh!

LeonarddeR left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

XLTechie commented May 1, 2024 via email

Uh oh!

cary-rowen commented May 2, 2024

Uh oh!

CyrilleB79 commented May 2, 2024

Uh oh!

cary-rowen commented May 5, 2024

Uh oh!

AppVeyorBot commented May 5, 2024

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

seanbudd commented May 6, 2024

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cary-rowen commented May 14, 2024

Uh oh!

cary-rowen commented May 16, 2024

Uh oh!

Adriani90 commented May 17, 2024

Uh oh!

hwf1324 commented May 17, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

cary-rowen commented Apr 30, 2024 •

edited

Loading

hwf1324 commented May 17, 2024 •

edited

Loading