Skip to content

Fix magnifier tracking vertical lists#19733

Merged
seanbudd merged 2 commits into
nvaccess:masterfrom
France-Travail:fix/MagnifierFollow
Mar 10, 2026
Merged

Fix magnifier tracking vertical lists#19733
seanbudd merged 2 commits into
nvaccess:masterfrom
France-Travail:fix/MagnifierFollow

Conversation

@Boumtchack

@Boumtchack Boumtchack commented Mar 2, 2026

Copy link
Copy Markdown
Contributor

Link to issue number:

Fixes #19635

Summary of the issue:

When using Ctrl+Alt+Arrow to navigate vertical list, both the system focus (row) and the navigator object (cell) change simultaneously.

In getCurrentFocusCoordinates(), system focus had priority 3 and navigator had priority 4.
As a result, the magnifier always followed the row instead of the active cell.

Description of user facing changes:

  • Magnifier should follow properly on (Ctrl+Alt+Arrow).
  • No behavior change for:
    • Tab / normal focus navigation
    • Caret navigation (browse mode)
    • Numpad navigator movement

Description of developer facing changes:

focusManager.py

  • Inverted priorities 3 and 4 in getCurrentFocusCoordinates():

    • If both system focus and navigator change, navigator wins.
    • System focus only wins if navigator did not move.

Description of development approach:

The fix is limited to priority inversion to align behavior with expected table navigation semantics.

This ensures:

  • Normal focus changes remain unchanged.
  • Caret-only movement remains unchanged.
  • Navigator-only movement remains unchanged.
  • Simultaneous changes correctly favor the navigator (table cell case).

Testing strategy:

  • Added new test case.

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 Boumtchack marked this pull request as ready for review March 3, 2026 12:55
@Boumtchack Boumtchack requested a review from a team as a code owner March 3, 2026 12:55
@Boumtchack Boumtchack requested review from Copilot and seanbudd March 3, 2026 12:55

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Adjusts NVDA magnifier focus-tracking behavior so that during table cell navigation (e.g., Ctrl+Alt+Arrow), the magnifier follows the active cell (navigator/review position) rather than the focused row.

Changes:

  • Inverts focus-tracking priority in FocusManager.getCurrentFocusCoordinates() to prefer navigator changes over system focus changes.
  • Updates/extends unit tests to cover the “both system focus and navigator changed” scenario.

Reviewed changes

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

File Description
source/_magnifier/utils/focusManager.py Reorders focus coordinate selection so navigator/review changes take precedence over system focus changes.
tests/unit/test_magnifier/test_focusManager.py Adds a test case validating navigator precedence when both navigator and system focus change.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread source/_magnifier/utils/focusManager.py Outdated
Comment thread source/_magnifier/utils/focusManager.py
@Boumtchack Boumtchack marked this pull request as draft March 3, 2026 14:45
@seanbudd seanbudd self-assigned this Mar 5, 2026
@seanbudd seanbudd changed the title Focus Logic changed Fix magnifier tracking vertical lists Mar 5, 2026
@seanbudd seanbudd added the conceptApproved Similar 'triaged' for issues, PR accepted in theory, implementation needs review. label Mar 5, 2026
@seanbudd seanbudd changed the title Fix magnifier tracking vertical lists Fix magnifier tracking lists and tables Mar 5, 2026
@CyrilleB79

Copy link
Copy Markdown
Contributor

@seanbudd for clarity, I think this fix is only about list views, isn't it? (no need to mention tables in the PR title)

@seanbudd

Copy link
Copy Markdown
Member

The PR description seems to refer to tables, can that be updated if thats not accurate?

@Boumtchack Boumtchack changed the title Fix magnifier tracking lists and tables Fix magnifier tracking vertical lists Mar 10, 2026
@Boumtchack Boumtchack marked this pull request as ready for review March 10, 2026 09:01

@seanbudd seanbudd left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks @Boumtchack

@seanbudd seanbudd merged commit 7ffb879 into nvaccess:master Mar 10, 2026
39 of 41 checks passed
@github-actions github-actions Bot added this to the 2026.2 milestone Mar 10, 2026
@Boumtchack Boumtchack deleted the fix/MagnifierFollow branch April 21, 2026 13:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

conceptApproved Similar 'triaged' for issues, PR accepted in theory, implementation needs review.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Magnifier does not follow correctly when navigating vertically in list views

4 participants