Skip to content

Fix UIA text unit crash and Word sentence-nav boundary wraparound - #20498

Merged
seanbudd merged 1 commit into
nvaccess:betafrom
LeonarddeR:uiaSentenceFixes
Jul 27, 2026
Merged

Fix UIA text unit crash and Word sentence-nav boundary wraparound#20498
seanbudd merged 1 commit into
nvaccess:betafrom
LeonarddeR:uiaSentenceFixes

Conversation

@LeonarddeR

Copy link
Copy Markdown
Collaborator

Link to issue number:

Follow-up to #19367.
based on beta because it fixes a feature introduced in said branch that has not yet been released.

Summary of the issue:

While wiring native Word sentence navigation into WordDocumentTextInfo.expand()/move() for UNIT_SENTENCE (related to #20467), two issues surfaced:

  1. UIATextInfo.expand()/move() raise an uncaught KeyError for any NVDA text unit UIA has no mapping for, instead of a NotImplementedError, which is the contract TextInfo.expand()/move() callers rely on for a graceful no-op. UNIT_SENTENCE is one such unmapped unit, so calling expand()/move() with it on a UIA TextInfo crashed instead of raising cleanly.
  2. Word's native MoveBySentence/ExpandToEnclosingSentence UIA extensions silently wrap around at document boundaries instead of stopping. Moving forward from the last sentence (or expanding a collapsed range at the very end of the document) teleports the range to the start of the document rather than staying put.

Description of user facing changes:

Description of developer facing changes:

  • Added UIAHandler.getUIAUnitFromNVDAUnit(), which raises NotImplementedError instead of letting a KeyError escape NVDAUnitsToUIAUnits lookups. UIATextInfo.expand()/move() now use it. This is a general robustness fix for the UIA TextInfo API, strictly spoken independent of sentence navigation but definitely a show stopper for Restore sentence as the default say all reading unit, with a setting to read by paragraph or line #20467. It feels more appropriate to fix this in the current PR instead of in Restore sentence as the default say all reading unit, with a setting to read by paragraph or line #20467.
  • UIAHandler.remote.msWord_moveTextRangeBySentence() is split into msWord_textRange_moveBySentence(), msWord_textRange_moveEndpointBySentence(), and msWord_textRange_expandToEnclosingSentence().
  • As msWord_moveTextRangeBySentence is UIA-remote-ops-internal API that has only ever shipped in 2026.2 betas (no stable release), and this branch targets beta, the rename drops the function outright rather than keeping a deprecated back-compat shim.

Description of development approach:

  • Wired native Word sentence support directly into WordDocumentTextInfo.expand()/move() for UNIT_SENTENCE. The sentence-navigation script now goes through this TextInfo API instead of calling the remote op directly.
  • Added a boundary guard in UIAHandler.remote.msWord_textRange_expandToEnclosingSentence(): a collapsed range at the very end of the document is returned unchanged instead of being expanded to the first sentence in the document.

Testing strategy:

  • Added unit tests for getUIAUnitFromNVDAUnit() (mapped and unmapped units).
  • Manually verified in Word: moving by sentence (alt+downArrow/alt+upArrow) at the start/end of a document stops instead of wrapping.

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.

UIATextInfo.expand()/move() raised an uncaught KeyError for any NVDA
text unit UIA has no mapping for (e.g. sentence), instead of the
NotImplementedError browse mode already expects for graceful no-ops.
Added UIAHandler.getUIAUnitFromNVDAUnit() to convert the lookup
failure properly.

Separately, Word's native MoveBySentence/ExpandToEnclosingSentence
UIA extensions silently wrap around at document boundaries rather
than stopping, which would teleport the caret to the opposite end of
the document. Added a boundary guard to
msWord_textRange_expandToEnclosingSentence and wired native sentence
support into WordDocumentTextInfo.expand()/move() so browse mode
gets real Word sentence navigation instead of a silent no-op.

msWord_moveTextRangeBySentence is split into
msWord_textRange_moveBySentence, msWord_textRange_moveEndpointBySentence,
and msWord_textRange_expandToEnclosingSentence. As the old name has
only ever shipped in 2026.2 betas, it is removed outright rather than
kept as a deprecated alias.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@LeonarddeR
LeonarddeR marked this pull request as ready for review July 14, 2026 21:26
@LeonarddeR
LeonarddeR requested a review from a team as a code owner July 14, 2026 21:26
@LeonarddeR
LeonarddeR requested review from SaschaCowley and Copilot and removed request for Copilot July 14, 2026 21:26
@LeonarddeR

Copy link
Copy Markdown
Collaborator Author

@codeofdusk Could you please share your thoughts here?

@seanbudd seanbudd added this to the 2026.2 milestone Jul 14, 2026
@seanbudd

Copy link
Copy Markdown
Member

If there's no user facing changes, do we need to take this PR to beta? It seems complicated and risky.

@LeonarddeR

LeonarddeR commented Jul 15, 2026

Copy link
Copy Markdown
Collaborator Author

If there's no user facing changes,

That is not correct. This fixes an issue where sentence nav wraps at the bottom of a document. So currently in Word, if you have a document with Hello. Cheese. Test., word will do:

  • Hello.
  • Cheese.
  • Test.
  • Blank
  • Hello.
  • Cheese.
  • Etc.

Do we need to take this PR to beta? It seems complicated and risky.

The reason why I filed this against beta initially is that it changes the msWord_moveTextRangeBySentence return type, which is an API breaking change. The current implementation of ``msWord_moveTextRangeBySentence` is not compatible with thext info move, because it also auto expands.

@seanbudd What would you suggest to limit the change to a minimum? E.g. making msWord_moveTextRangeBySentence private? Deprecating it in 2026.3 feels odd.

@SaschaCowley

Copy link
Copy Markdown
Member

This looks alright to me. @seanbudd are you happy accepting this to beta this late? Note that it contains what would be API breaking changes if introduced in a future release.

@seanbudd

Copy link
Copy Markdown
Member

Yes, let's merge this

@SaschaCowley SaschaCowley added the conceptApproved Similar 'triaged' for issues, PR accepted in theory, implementation needs review. label Jul 21, 2026
@seanbudd
seanbudd merged commit d9f27fe into nvaccess:beta Jul 27, 2026
36 of 39 checks passed
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.

4 participants