Skip to content

Restore sentence as the default say all reading unit, with a setting to read by paragraph or line - #20467

Merged
seanbudd merged 15 commits into
nvaccess:masterfrom
LeonarddeR:sayAllReadingUnit
Aug 3, 2026
Merged

Restore sentence as the default say all reading unit, with a setting to read by paragraph or line#20467
seanbudd merged 15 commits into
nvaccess:masterfrom
LeonarddeR:sayAllReadingUnit

Conversation

@LeonarddeR

@LeonarddeR LeonarddeR commented Jul 8, 2026

Copy link
Copy Markdown
Collaborator

Link to issue number:

Related to #13420, #9179 and #13971
Fixes #13829 by making paragraph available as an additional unit, so say all can send whole paragraphs to the synthesizer.

Summary of the issue:

For rich text controls where UNIT_READINGCHUNK was previously mapped to line (ITextDocumentTextInfo, e.g. WordPad, NVDA's log viewer, and third-party apps such as Paperback), say all's caret/review position hardly updates when word wrap is off and a paragraph spans many visual lines — the position only advances once the whole (very long) paragraph line is read. This was introduced by #13971, which changed the reading unit from sentence to line for these controls to fix caret tracking in other apps (#13420, #9179).

This PR effectively reverts the default introduced by #13971: say all now reads by sentence again by default wherever sentence boundaries are supported, restoring smoother caret tracking in rich edit controls with long paragraphs. To avoid regressing the caret-tracking fix #13971 provided for apps/languages without usable sentence boundaries, the choice is no longer hardcoded: a new speech setting lets users switch back to line-based reading, re-enabling the behavior @lukaszgo1 introduced in #13971 where they need it. A paragraph unit is also available, which sends larger chunks of text to the synthesizer so it can determine sentence boundaries (and e.g. abbreviations like "Mr.") itself (#13829).

Description of user facing changes:

  • New "Say all reads by" combo box in the Speech settings panel, with three options:
  • Say all now reads by sentence by default in Rich Edit controls such as WordPad and NVDA's log viewer.

Description of developer facing changes:

  • Added SayAllReadingUnitFlag feature flag (config.featureFlagEnums) with a [speech] sayAllReadingUnit configSpec entry, behaviorOfDefault="sentence".
  • Added TextInfo.unit_readingChunk, resolving textInfos.UNIT_READINGCHUNK to UNIT_SENTENCE/UNIT_PARAGRAPH/UNIT_LINE based on the flag's calculated value; cached per core pump (_cache_unit_readingChunk) to avoid repeated config lookups in the say all loop.
  • Added TextInfo._resolveReadingChunkUnit, used by winword.py/edit.py expand/move to substitute the resolved unit for UNIT_READINGCHUNK before their static unit-map lookup; the now-unreachable UNIT_READINGCHUNK entries were removed from those maps.
  • OffsetsTextInfo._getReadingChunkOffsets dispatches self.unit_readingChunk through the existing _getUnitOffsets, falling back to line offsets if the resolved unit raises NotImplementedError.
  • UIA-based TextInfos are unaffected: UIAHandler.NVDAUnitsToUIAUnits hardcodes UNIT_READINGCHUNK to TextUnit_Line, since UIA's TextPattern has no sentence text unit. This means modern UIA Word (NVDAObjects/UIA/wordDocument.py) already reads by line for say all regardless of this flag (the paragraph option is likewise ignored there for now); only legacy Word Object Model Word (winword.py, wdSentence) is driven by the flag.

Description of development approach:

Introduced the reading unit as a feature flag rather than hardcoding a default, so the sentence/line choice from #13971 remains available per user preference instead of being an all-or-nothing global default. SayAllReadingUnitFlag avoids a module-level import textInfos in featureFlagEnums.py, since that module is imported during config package init, before languageHandler installs the _ gettext builtin, and textInfos transitively calls _() at import time.

Testing strategy:

  • Added unit tests for _getReadingChunkOffsets covering the sentence, paragraph, line, and unsupported-unit-falls-back-to-line cases.
  • Manually verified say all in WordPad and NVDA's log viewer with both settings.

Known issues with pull request:

This PR defaults the flag to sentence, effectively reverting #13971's default. If reviewers decide line should remain the default (per #13971's rationale for #13420/#9179), the fix is a one-line change: flip behaviorOfDefault in the sayAllReadingUnit configSpec entry (source/config/configSpec.py) from "sentence" to "line". Note this would also move Word Object Model Word (winword.py, legacy Word) to line-based reading — it currently reads by sentence unconditionally on master (wdSentence, unaffected by #13971) and is now driven by the same flag as WordPad/log viewer. I consider this acceptable: a single uniform default across all UNIT_READINGCHUNK-capable TextInfos is preferable to Word Object Model Word and rich-edit controls disagreeing on reading unit by default. Modern UIA Word is unaffected by the default either way — it already reads by line unconditionally (see developer facing changes).

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.

LeonarddeR and others added 6 commits July 8, 2026 19:43
Add a SayAllReadingUnitFlag feature flag controlling the text unit
say all advances by (the reading chunk), with an explicit DEFAULT
whose behavior is sentence, plus the [speech] configSpec entry.

Enum values are inlined string literals ('sentence'/'line') equal to
textInfos.UNIT_SENTENCE/UNIT_LINE. A module-level 'import textInfos'
was avoided: featureFlagEnums is imported during config package init,
before languageHandler installs the '_' gettext builtin, and textInfos
pulls in controlTypes.formatFields which calls _() at module load,
which would crash startup.
…le TextInfos

Make each sentence-capable TextInfo resolve UNIT_READINGCHUNK to the
effective value of the sayAllReadingUnit feature flag (sentence or line),
keeping line as the automatic fallback where sentence is unsupported:

- textInfos/offsets.py: _getReadingChunkOffsets tries sentence offsets when
  the flag resolves to sentence, falling back to line on NotImplementedError.
- NVDAObjects/window/winword.py and edit.py: substitute the calculated unit
  for UNIT_READINGCHUNK before the static unit-map lookup in expand/move.
- NVDAObjects/IAccessible/MSHTML.py: replace the READINGCHUNK->SENTENCE
  remaps in expand/move with the calculated unit; preserve the existing
  LINE->SENTENCE behaviour in move.

Add unit tests for _getReadingChunkOffsets covering sentence, line, and the
NotImplementedError line fallback.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Add a FeatureFlagCombo for the speech.sayAllReadingUnit feature flag to
SpeechSettingsPanel, with context help binding and onSave wiring.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Add a userGuide entry (anchored SpeechSettingsSayAllReadingUnit for
context help) and a changes.md New Features entry.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…ies, guard enum

- MSHTML: revert the READINGCHUNK resolution. IHTMLTxtRange::move cannot move
  by line (the original mapped both READINGCHUNK and LINE to sentence for that
  reason), so honouring a "line" flag value there made say all's move soft-fail
  and stop after the first chunk. Legacy MSHTML keeps reading by sentence.
- winword.py / edit.py: remove the now-unreachable UNIT_READINGCHUNK entries from
  the static unit maps (READINGCHUNK is resolved to sentence/line before the map
  lookup in expand/move).
- Add a unit test guarding that SayAllReadingUnitFlag's inlined "sentence"/"line"
  values stay equal to textInfos.UNIT_SENTENCE/UNIT_LINE.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@LeonarddeR

LeonarddeR commented Jul 8, 2026

Copy link
Copy Markdown
Collaborator Author

Also mentioning @cary-rowen here, since he was particularly involved with #13420 debugging.
Personally I'm using Paperback a lot lately and it is a real thorn in my eye that for long paragraphs, caret doesn't follow per sentence. I tend to disable word wrap when studying because it makes skimming a load more difficult.
Would the solution filed here be acceptable for you?
I have considered filing an issue first, but given how efficient AI is these days, I figured that it would be more helpful to provide a prototype so we can test the implementation while discussing😉

- new SayAllReadingUnitFlag.PARAGRAPH ("Prefer paragraph"), resolved
  via unit_readingChunk with line fallback where unsupported
- hoist duplicated UNIT_READINGCHUNK resolution from edit/winword
  expand/move into TextInfo._resolveReadingChunkUnit
- _getReadingChunkOffsets reuses _getUnitOffsets dispatch
- cache unit_readingChunk per core pump to avoid config reads in the
  say all hot loop
"Prefer sentence" etc. read oddly after the "Say all reads by" label;
options now complete the label as a sentence. Paragraph and line need
no qualifier since they are always supported.
@LeonarddeR LeonarddeR changed the title Restore sentence as the default say all reading unit, with a setting to prefer line Restore sentence as the default say all reading unit, with a setting to read by paragraph or line Jul 10, 2026
@LeonarddeR
LeonarddeR marked this pull request as ready for review July 10, 2026 19:23
@LeonarddeR
LeonarddeR requested review from a team as code owners July 10, 2026 19:23

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

This PR reintroduces sentence-based “Say All” as the default reading unit (where supported) and makes the reading-chunk unit user-configurable via a new Speech setting, including a paragraph option to allow larger chunks to be sent to synthesizers.

Changes:

  • Added a new “Say all reads by” Speech setting (feature flag-backed) with options for sentence (where possible), paragraph, and line.
  • Updated TextInfo implementations to resolve UNIT_READINGCHUNK dynamically based on the configured reading unit, including support in legacy Word (winword) and Rich Edit (ITextDocument).
  • Added unit tests for reading-chunk offsets resolution and fallback-to-line behavior when a unit isn’t implemented.

Reviewed changes

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

Show a summary per file
File Description
user_docs/en/userGuide.md Documents the new Speech setting in the User Guide.
user_docs/en/changes.md Adds a changelog entry for the new setting / default behavior change.
tests/unit/test_textInfos.py Adds unit tests for reading-chunk offset behavior across configured units.
source/textInfos/offsets.py Resolves reading-chunk offsets via the configured unit with fallback to line when unsupported.
source/textInfos/init.py Introduces unit_readingChunk and unit resolution helper for UNIT_READINGCHUNK.
source/NVDAObjects/window/winword.py Resolves UNIT_READINGCHUNK before mapping units to Word units.
source/NVDAObjects/window/edit.py Resolves UNIT_READINGCHUNK before mapping units to TOM units.
source/config/configSpec.py Adds the new [speech] sayAllReadingUnit feature-flag config entry.

Comment thread source/textInfos/offsets.py
Comment thread user_docs/en/userGuide.md
Comment thread user_docs/en/changes.md
@SaschaCowley SaschaCowley added the conceptApproved Similar 'triaged' for issues, PR accepted in theory, implementation needs review. label Jul 21, 2026
seanbudd pushed a commit that referenced this pull request Jul 27, 2026
…0498)

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:

    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.
    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:

    Moving by sentence in Word (alt+downArrow/alt+upArrow) at the start/end of a document now stops there instead of wrapping around to the opposite end.
    No new user-facing feature: this is a fix to code introduced in UI Automation in Microsoft Word: Use move by sentence custom pattern when available instead of the legacy model #19367, which itself has no changes.md entry as it was an internal backend swap.

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.

@SaschaCowley SaschaCowley 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.

Two very minor things

Comment thread source/textInfos/__init__.py Outdated
Comment thread source/textInfos/offsets.py
@SaschaCowley
SaschaCowley marked this pull request as draft July 27, 2026 06:58
Copilot AI review requested due to automatic review settings July 27, 2026 08:24

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

Copilot reviewed 10 out of 10 changed files in this pull request and generated 1 comment.

Comments suppressed due to low confidence (1)

source/textInfos/offsets.py:574

  • _getReadingChunkOffsets currently catches NotImplementedError around both resolving the configured reading-chunk unit and computing offsets. If unit_readingChunk raises NotImplementedError (e.g. due to an unexpected flag value), this will be silently masked and fall back to line offsets, making configuration/logic errors hard to detect.
		try:
			return self._getUnitOffsets(self.unit_readingChunk, offset)
		except NotImplementedError:
			return self._getLineOffsets(offset)

Comment thread source/textInfos/__init__.py Outdated
An unrecognised `sayAllReadingUnit` value is a bad value rather than an
unimplemented operation, matching `_getUnitOffsets`, which already raises
`ValueError` for an unknown unit.

`_getReadingChunkOffsets` now resolves the unit outside its `try`, so its
fallback to line offsets only covers units the TextInfo does not implement.
Copilot AI review requested due to automatic review settings July 27, 2026 09:08
@LeonarddeR
LeonarddeR marked this pull request as ready for review July 27, 2026 09:11

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

Copilot reviewed 10 out of 10 changed files in this pull request and generated 1 comment.

Comments suppressed due to low confidence (1)

user_docs/en/userGuide.md:2307

  • The descriptions for the "Paragraph" and "Line" options imply they apply universally ("reads paragraph by paragraph" / "always reads line by line"). In practice, some TextInfo implementations override these units (e.g. MSHTML maps line/readingChunk to sentence), and others hardcode readingChunk to line regardless of user choice. To avoid user-facing documentation being incorrect, it would be better to qualify these options as "where supported" / "otherwise falls back to line" (similar to the sentence option).
When set to "Sentence where possible", NVDA reads sentence by sentence in controls and documents that support sentence boundaries, and automatically falls back to reading by line where sentence boundaries are not supported.
When set to "Paragraph", NVDA reads paragraph by paragraph.
When set to "Line", NVDA always reads line by line.

Comment thread tests/unit/test_textInfos.py
_LineOnlyTextInfo inherited OffsetsTextInfo._getParagraphOffsets, which
delegates to _getLineOffsets, so the paragraph fallback test passed via the
paragraph-supported path and could not detect a regression in the
NotImplementedError handling of _getReadingChunkOffsets.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings July 27, 2026 16:07

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

Copilot reviewed 10 out of 10 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (2)

user_docs/en/userGuide.md:2307

  • The description of the "Paragraph" option is absolute, but (as with sentences) not all controls/TextInfo implementations support paragraph boundaries for say all reading chunks. The docs should qualify this (e.g. "where supported" / fallback to line) to avoid promising behavior that NVDA may not be able to deliver in some contexts.
This combo box lets you choose the unit of text that say all (continuous reading) advances by.
When set to "Sentence where possible", NVDA reads sentence by sentence in controls and documents that support sentence boundaries, and automatically falls back to reading by line where sentence boundaries are not supported.
When set to "Paragraph", NVDA reads paragraph by paragraph.
When set to "Line", NVDA always reads line by line.

user_docs/en/changes.md:27

  • The PR description explicitly mentions fixing #13829 (paragraph-sized chunks to improve synthesizer sentence handling), but this change log entry doesn’t reference it. Please include #13829 in the issue list for traceability.
* A new "Say all reads by" speech setting lets you choose whether say all reads by sentence, paragraph or line; say all now reads by sentence by default where supported. (#13420, #9179, #13971, @LeonarddeR)

Copilot AI review requested due to automatic review settings July 30, 2026 00:12

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.

Copilot was unable to review this pull request because the user who requested the review is ineligible. To be eligible to request a review, you need a paid Copilot license, or your organization must enable Copilot code review.

@SaschaCowley
SaschaCowley enabled auto-merge (squash) July 30, 2026 00:13

@Qchristensen Qchristensen 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.

User Guide changes are all fine, though I was wondering if it was worth a line of clarification on what this feature does? Just something simple like:

"This setting affects how frequently the text caret and view move during say all, and can also affect intonation in long blocks of text."

(I think an article in the In-Process blog could go into more detail, but I can see some users being confused by "Doesn't say all read from the current point to the end of the document? So what do you mean by moving by line or paragraph etc?)

Comment thread user_docs/en/userGuide.md
auto-merge was automatically disabled July 31, 2026 07:20

Head branch was pushed to by a user without write access

Copilot AI review requested due to automatic review settings July 31, 2026 07:20

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

Copilot reviewed 10 out of 10 changed files in this pull request and generated no new comments.

Suppressed comments (2)

user_docs/en/userGuide.md:2316

  • The documentation for the "Paragraph" option implies say all will always advance by paragraph, but the implementation falls back to line offsets when paragraph boundaries aren't implemented (e.g. OffsetsTextInfo._getReadingChunkOffsets catches NotImplementedError and returns line offsets). Consider documenting this fallback so users know what to expect in controls without paragraph support.
When set to "Paragraph", NVDA reads paragraph by paragraph.

user_docs/en/changes.md:29

  • Minor grammar: add a serial comma in the list "sentence, paragraph or line".
* A new "Say all reads by" speech setting lets you choose whether say all reads by sentence, paragraph or line; say all now reads by sentence by default where supported. (#13420, #9179, #13971, @LeonarddeR)

@Qchristensen Qchristensen 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.

Reads well, thanks!

Copilot AI review requested due to automatic review settings August 3, 2026 06:19
@seanbudd
seanbudd merged commit da4cdbc into nvaccess:master Aug 3, 2026
10 checks passed
@github-actions github-actions Bot added this to the 2026.3 milestone Aug 3, 2026

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

Copilot reviewed 10 out of 10 changed files in this pull request and generated no new comments.

Suppressed comments (3)

source/textInfos/offsets.py:581

  • _getReadingChunkOffsets is annotated/documented to return a tuple, but it currently returns whatever _getUnitOffsets/_getLineOffsets return (which can be a list in this class). Converting to tuple here keeps the runtime behavior consistent with the type hint and avoids callers having to handle mixed list/tuple return types.
		unit = self.unit_readingChunk
		try:
			return self._getUnitOffsets(unit, offset)
		except NotImplementedError:
			return self._getLineOffsets(offset)

user_docs/en/userGuide.md:2310

  • The User Guide anchor includes the settings category ("SpeechSettings"). NVDA’s User Guide anchors are intended to remain stable even if a setting moves categories; introducing a category-specific anchor now risks locking in a misleading anchor name.
##### Say all reads by {#SpeechSettingsSayAllReadingUnit}

source/gui/settingsDialogs.py:1765

  • If the User Guide anchor is changed to be category-independent (e.g. "SayAllReadingUnit"), this help topic ID should be updated to match so that F1 help continues to jump to the correct section.
		self.bindHelpEvent("SpeechSettingsSayAllReadingUnit", self.sayAllReadingUnitCombo)

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.

Say all pauses in the wrong places

5 participants