Skip to content

fix magnifier follow modes bug#19992

Merged
seanbudd merged 3 commits intonvaccess:masterfrom
France-Travail:fix/followModesIssues
Apr 24, 2026
Merged

fix magnifier follow modes bug#19992
seanbudd merged 3 commits intonvaccess:masterfrom
France-Travail:fix/followModesIssues

Conversation

@Boumtchack
Copy link
Copy Markdown
Contributor

@Boumtchack Boumtchack commented Apr 21, 2026

Link to issue number:

fixes #19991
fixes #19990

Summary of the issue:

The magnifier follow-mode behavior was inconsistent: disabling follow modes could still cause view movement, toggling follow settings could trigger an immediate jump, and “Toggle all follow settings” did not provide the expected disable/restore flow.

Description of user facing changes:

The magnifier view now remains stable when follow modes are toggled, stays frozen when all follow modes are disabled, and “Toggle all follow settings” now works as a true two-step action: disable all first, then restore the previous follow configuration.

Description of developer facing changes:

Focus tracking logic in the magnifier now preserves and reuses the last reported coordinates when no enabled follow source produces a new event, and follow-setting toggle commands no longer force an immediate focus refresh. The all-follow toggle state handling was updated to implement disable-all and restore semantics.

Description of development approach:

The fix was implemented by adjusting the focus source resolution flow to avoid fallback-driven movement, removing forced update calls from follow toggle commands, and aligning global follow-toggle state transitions with the intended UX behavior (disable all -> restore previous states).

Testing strategy:

manual

Known issues with pull request:

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.

@Boumtchack
Copy link
Copy Markdown
Contributor Author

@CyrilleB79

@CyrilleB79
Copy link
Copy Markdown
Contributor

In #19990, I was suggesting to add another tracker, namely "Follow system cursor". I have not yet tested this PR, but it seems that it's not the solution you have chosen. Can you clarify here how the magnified view reacts or doesn't react to system cursor moves (or browse mode cursor), depending on the various trackers ("Follow X" options)?

@seanbudd
Copy link
Copy Markdown
Member

@Boumtchack - can we please focus on the existing PRs you have open? It would be really valuable for the project to finish all the started work before taking on new tasks.

@seanbudd seanbudd closed this Apr 22, 2026
@CyrilleB79
Copy link
Copy Markdown
Contributor

@seanbudd, this PR is a fix-up of PR #19739 that was merged just yesterday. It would make sense to finish the work of #19739. Can you reconsider the reason why you closed this and reopen it?

@seanbudd
Copy link
Copy Markdown
Member

Understood - happy to reopen.

@seanbudd seanbudd reopened this Apr 22, 2026
@seanbudd
Copy link
Copy Markdown
Member

should this be marked as ready for review?

@Boumtchack
Copy link
Copy Markdown
Contributor Author

@CyrilleB79

Current behavior is:

System caret / browse mode cursor movement is handled by Follow system focus (it reads caret position first, then falls back to focused object location).
If Follow system focus = ON, caret/browse cursor moves can move the magnified view.
If Follow system focus = OFF, caret/browse cursor moves do not move the view.
The view can still move from other enabled trackers:
Follow mouse
Follow review cursor
Follow navigator object
Toggling any follow option (including “all follow settings”) no longer causes an immediate jump; the view stays where it is until a new event from an enabled tracker occurs.
If all trackers are disabled, the view remains frozen on the last displayed position.

@Boumtchack
Copy link
Copy Markdown
Contributor Author

Also, if no follow option is selected, the magnifier will not move, since “Toggle all” now behaves as “disable all / restore previously enabled options.”
If we want a true “all on / all off” toggle, we would need a second gesture to restore default settings, and I’m not sure that is the direction we want to take.

@Boumtchack
Copy link
Copy Markdown
Contributor Author

Adding a dedicated “Follow system cursor” tracker could still be useful to give users finer control by separating caret/browse-cursor tracking from general system-focus tracking, but it would also introduce an additional setting and gesture, which may increase complexity.

@CyrilleB79
Copy link
Copy Markdown
Contributor

Also, if no follow option is selected, the magnifier will not move, since “Toggle all” now behaves as “disable all / restore previously enabled options.”

Yes, that would be nice. That's exactly what I do in my Windows Magnifier add-on. Though, testing tis PR (commit defffcc), the toggle all still behaves as a true all/off toggle... Could you double check?

If we want a true “all on / all off” toggle, we would need a second gesture to restore default settings, and I’m not sure that is the direction we want to take.

No, unless someone request it in the future; in this case this will be discussed in a separate issue.

Adding a dedicated “Follow system cursor” tracker could still be useful to give users finer control by separating caret/browse-cursor tracking from general system-focus tracking, but it would also introduce an additional setting and gesture, which may increase complexity.

OK. In any case, this PR is a fix for what had already validated and merged. In case we want to add a specific option for system cursor tracking, let's discuss it in a dedicated separate issue later.

@Boumtchack
Copy link
Copy Markdown
Contributor Author

Boumtchack commented Apr 22, 2026

Yes, that would be nice. That's exactly what I do in my Windows Magnifier add-on. Though, testing tis PR (commit defffcc), the toggle all still behaves as a true all/off toggle... Could you double check?

it should be working like this:
If Mouse is the only follow option enabled and you press "Toggle All", all options get disabled. Pressing it again restores only Mouse, the others stay disabled.

If all options are enabled and you press "Toggle All", all get disabled. Pressing again re-enables all of them. This looks like a true on/off toggle, but it's just because all were enabled to begin with, this is likely what you observed during testing.

can you tell me again how you tested it ?

@CyrilleB79
Copy link
Copy Markdown
Contributor

h> > Yes, that would be nice. That's exactly what I do in my Windows Magnifier add-on. Though, testing tis PR (commit defffcc), the toggle all still behaves as a true all/off toggle... Could you double check?

it should be working like this: If Mouse is the only follow option enabled and you press "Toggle All", all options get disabled. Pressing it again restores only Mouse, the others stay disabled.

If all options are enabled and you press "Toggle All", all get disabled. Pressing again re-enables all of them. This looks like a true on/off toggle, but it's just because all were enabled to begin with, this is likely what you observed during testing.

can you tell me again how you tested it ?

I have just re-tested (on commit 109c196) and all is working as expected, i.e. as you describe. Sorry for inconvenience, I had probably tested badly the first time.

@Boumtchack Boumtchack marked this pull request as ready for review April 22, 2026 12:30
@Boumtchack Boumtchack requested a review from a team as a code owner April 22, 2026 12:30
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Fixes regressions in NVDA magnifier “follow modes” behavior so toggling follow settings no longer causes unexpected view movement, and disabling all follow modes properly freezes the view until a followed source actually changes.

Changes:

  • Add “last reported” coordinate tracking in FocusManager to preserve/freeze the magnifier position when no enabled follow source produces a new event.
  • Update “toggle all follow settings” semantics to a true disable-all → restore-previous flow.
  • Adjust magnifier follow-mode unit tests to cover freeze/restore behavior.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

File Description
source/_magnifier/utils/focusManager.py Tracks last reported coordinates and returns them when no eligible follow update occurs (prevents jumps on toggles / all-disabled freeze).
source/_magnifier/config.py Changes toggleAllFollowStates to disable all first, then restore saved states on next toggle.
source/_magnifier/commands.py Removes forced immediate focus refresh when toggling follow settings; updates “toggle all” messaging.
tests/unit/test_magnifier/test_focusManager.py Adds/updates tests to validate freeze behavior when follow modes are disabled or toggled.

Comment thread source/_magnifier/utils/focusManager.py
Comment thread source/_magnifier/commands.py
@seanbudd
Copy link
Copy Markdown
Member

does this close the issues mentioned in the PR description? If so, make sure to prefix each issue number with "fixes"

@seanbudd seanbudd changed the title hotfix follow modes bug fix magnifier follow modes bug Apr 23, 2026
@CyrilleB79
Copy link
Copy Markdown
Contributor

Yes #19990 and #19991 are fixed in this PR according to the initial description:

The magnifier view now remains stable when follow modes are toggled, stays frozen when all follow modes are disabled

I have also performed tests and can confirm that this PR fixes these two issues.

@seanbudd seanbudd merged commit 0e1152e into nvaccess:master Apr 24, 2026
35 of 37 checks passed
@github-actions github-actions Bot added this to the 2026.2 milestone Apr 24, 2026
@Boumtchack Boumtchack deleted the fix/followModesIssues branch May 6, 2026 09:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Magnifier - Toggling follow modes should not update the view Magnifier - When all follow modes are disabled, the view may still be updated

4 participants