Skip to content

Fix OffsetTextInfo.collapse() and OffsetTextInfo.move() #18348

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open

Fix OffsetTextInfo.collapse() and OffsetTextInfo.move() #18348

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
6e75803
Fix OffsetTextInfo.collapse() and OffsetTextInfo.move()
mltony Jun 26, 2025
012f6aa
Fix system test
mltony Jun 27, 2025
a8575c3
Address comment
mltony Jun 27, 2025
bae89c0
Pre-commit auto-fix
pre-commit-ci[bot] Jun 27, 2025
8247486
Update source/textInfos/offsets.py
mltony Jun 28, 2025
08bb02c
Update user_docs/en/changes.md
mltony Jul 10, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 0 additions & 10 deletions source/NVDAObjects/window/scintilla.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -313,16 +313,6 @@ def _getCharacterOffsets(self, offset):
tempOffset -= 1
return [start, end]

def collapse(self, end: bool = False):
"""Before collapsing to end, if no text is selected, TextInfo is expanded to line.
This fixes a bug where next braille line command didn't move the cursor to the last empty line
in Notepad++ documents.
https://github.com/nvaccess/nvda/issues/17430
"""
if end and self.obj.makeTextInfo(textInfos.POSITION_SELECTION).isCollapsed:
self.expand(textInfos.UNIT_LINE)
super().collapse(end=end)


# The Scintilla NVDA object, inherists the generic MSAA NVDA object
class Scintilla(EditableTextWithAutoSelectDetection, Window):
Expand Down
8 changes: 6 additions & 2 deletions source/textInfos/offsets.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -648,7 +648,11 @@ def unitCount(self, unit):
else:
raise NotImplementedError

allowMoveToOffsetPastEnd = True #: move with unit_character can move 1 past story length to allow braille routing to end insertion point. (#2096)
allowMoveToOffsetPastEnd = True
"""
We can move 1 past story length to allow braille routing to end insertion point. (#2096)
Furthermore, review cursor is able to reach the last, empty line in some controls, like Scintilla. (#18348)
"""

def move(self, unit, direction, endPoint=None):
if direction == 0:
Expand All @@ -664,7 +668,7 @@ def move(self, unit, direction, endPoint=None):
count = 0
lowLimit = 0
highLimit = self._getStoryLength()
if self.allowMoveToOffsetPastEnd and unit == textInfos.UNIT_CHARACTER:
Copy link
Member

Choose a reason for hiding this comment

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

@jcsteh looks like you implemented this in 792074e. Do you remember if there was a reason to restrict this to navigation by character only? Was it just because that is what was strictly necessary to solve #2096?

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't remember the specifics. However, I'll point out that "the very last position" (and what is supported there) can differ between text implementations.

  1. For some non-editable implementations, the length might not be a valid start offset.
  2. Normally, length is considered part of the final line, not a blank line. That raises the question: does it actually make sense to move there by line or paragraph? Could that confuse some callers to behave like there is a blank line at the end when there in fact isn't?

I don't have answers to these points, but they need to be considered if they haven't already. Note also that I haven't read this entire thread, only skimmed it, so I might have missed something.

if self.allowMoveToOffsetPastEnd:
# #2096: There is often an uncounted character at the end of the text
# where the caret is placed to append text.
highLimit += 1
Expand Down
2 changes: 1 addition & 1 deletion tests/system/robot/chromeTests.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -927,7 +927,7 @@ def test_pr11606():
actualSpeech = _chrome.getSpeechAfterKey("end")
_asserts.strings_match(
actualSpeech,
"link",
"blank",
)
# Read the current line.
# Before pr #11606 the next line ("C D") would have been read.
Expand Down
2 changes: 2 additions & 0 deletions user_docs/en/changes.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,8 @@ Please refer to [the developer guide](https://www.nvaccess.org/files/nvda/docume
The several built-in table definitions are moved to the `__tables` module in that package. (#18194, @LeonarddeR)
* Microsoft SQL Server Management Studio now uses the Visual Studio app module, as SSMS is based on Visual Studio. (#18176, @LeonarddeR)
* NVDA will report Windows release revision number (for example: 10.0.26100.0) when `winVersion.getWinVer` is called and log this information at startup. (#18266, @josephsl)
* Fixed behavior of `TextInfo.collapse()` - previously it was moving TextInfo to the next paragraph in some cases. (#18320, @mltony)
* Fixed behavior of `OffsetTextInfo.move()` - previously it wouldn't move to the very end of the document unless moving by character. (#18348, @mltony)

#### Deprecations

Expand Down