Skip to content

Fix LibreOffice braille scrolling#19204

Merged
seanbudd merged 17 commits into
nvaccess:masterfrom
LeonarddeR:libreFix
Nov 17, 2025
Merged

Fix LibreOffice braille scrolling#19204
seanbudd merged 17 commits into
nvaccess:masterfrom
LeonarddeR:libreFix

Conversation

@LeonarddeR

@LeonarddeR LeonarddeR commented Nov 11, 2025

Copy link
Copy Markdown
Collaborator

Link to issue number:

Fixes #19152
Fixup for #18348,
Supersedes #19203, #19171

Summary of the issue:

In LibreOffice, it is not possible to use braille panning since #18348.

Description of user facing changes:

Panning works again in LibreOffice

Description of developer facing changes:

removed OffsetsTextInfo.allowMoveToOffsetPastEnd.

Description of development approach:

Rather than having one allowMoveToOffsetPastEnd class attribute, introduce a allowMoveToUnitOffsetPastEnd method that takes a unit.

Testing strategy:

Tested in libre office.

Known issues with pull request:

Deprecation pattern is a bit tricky here because allowMoveToOffsetPastEnd is a class level attribute. I think the pattern in this pr suffices because it was only used at the instance level.

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 LeonarddeR marked this pull request as ready for review November 11, 2025 20:45
@LeonarddeR LeonarddeR requested a review from a team as a code owner November 11, 2025 20:45

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 fixes a regression in LibreOffice braille scrolling introduced by PR #18348. The fix replaces the class-level attribute allowMoveToOffsetPastEnd with a unit-aware method allowMoveToUnitOffsetPastEnd to allow fine-grained control over when moving past the end of text is permitted based on the movement unit.

  • Introduced allowMoveToUnitOffsetPastEnd(unit) method to replace the boolean class attribute
  • Applied the new method to virtual buffers and compound document leaf text info classes
  • Added comprehensive unit tests for compound document text navigation

Reviewed Changes

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

Show a summary per file
File Description
user_docs/en/changes.md Documents the deprecation of allowMoveToOffsetPastEnd attribute
tests/unit/test_compoundDocuments.py Adds comprehensive unit tests for TreeCompoundTextInfo navigation across character, word, and line units
tests/unit/objectProvider.py Adds _isEqual method to PlaceholderNVDAObject for proper object comparison in tests
source/virtualBuffers/init.py Implements allowMoveToUnitOffsetPastEnd to return False for all units (no insertion point in virtual buffers)
source/textInfos/offsets.py Replaces allowMoveToOffsetPastEnd attribute with allowMoveToUnitOffsetPastEnd(unit) method and adds deprecation handling
source/compoundDocuments.py Introduces CompoundTextLeafTextInfo mixin that allows moving past end only for character and word units
source/appModules/soffice.py Applies CompoundTextLeafTextInfo mixin to SymphonyTextInfo for proper LibreOffice braille navigation

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

Comment thread tests/unit/test_compoundDocuments.py Outdated
Comment thread source/virtualBuffers/__init__.py Outdated
Comment thread tests/unit/objectProvider.py Outdated
Comment thread tests/unit/test_compoundDocuments.py
LeonarddeR and others added 3 commits November 12, 2025 07:34
@seanbudd

Copy link
Copy Markdown
Member

I think we would all feel a lot safer merging this if we get system tests for libre Office and notepad++. I'm not sure how feasible it is to install them on GitHub runners, but I think it's worth investigating as we've had this be a constant, testable problem.

@LeonarddeR

Copy link
Copy Markdown
Collaborator Author

Agreed there. Problem with LibreOffice is that it has a first start experience with tips at startup. I think they'll be getting in the way during tests, unless there's a workaround for that. May be @michaelweghorn can chime in here?

@michaelweghorn

Copy link
Copy Markdown
Contributor

Problem with LibreOffice is that it has a first start experience with tips at startup. I think they'll be getting in the way during tests, unless there's a workaround for that. May be @michaelweghorn can chime in here?

Tips (and other things like infobars, first start wizard,...) can be disabled via a LibreOffice option/setting. https://wiki.documentfoundation.org/Deployment_and_Migration#Post_deployment_configuration describes some ways those can be deployed/set before starting LibreOffice.
I can look into that in more detail once it becomes relevant - please just let me know.

@nvdaes

nvdaes commented Nov 13, 2025

Copy link
Copy Markdown
Collaborator

@LeonarddeR , I like this a lot.
In LibreOffice Writer, when we are in the last line, trying to scroll forward the cursor is not moved to the end, and this is not consistent with other applications. I didn't know this, but a fellow of a spanish mailing list has reported that some braille displays don't have routing keys, so sometimes maybe desirable to move the cursor to the end of the document, if possible. Can you add another parameter to return True if the object is the last one, or something like that, or should this be done in braille.py?

@LeonarddeR

Copy link
Copy Markdown
Collaborator Author

@nvdaes Does the issue you're describing also apply to NVDA 2025.3 and older? In that case, we definitely need to handle that separately, though it is slightly related.

@nvdaes

nvdaes commented Nov 13, 2025

Copy link
Copy Markdown
Collaborator

@LeonarddeR wrote:

Does the issue you're describing also apply to NVDA 2025.3 and older?

I tested this with NVDA 2025.3.1 and the issue, i.e., cursor is not moved to the end of the document from the last braille window when scrolling forward, is happening, so the described issue is not a regression. I was wondering if your new function should cover this case, to allow move the cursor to the end when no more text or objects are available.

@LeonarddeR

Copy link
Copy Markdown
Collaborator Author

I was wondering if your new function should cover this case, to allow move the cursor to the end when no more text or objects are available.

I might be able to do this.

@nvdaes

nvdaes commented Nov 13, 2025

Copy link
Copy Markdown
Collaborator

Leonard wrote:

I might be able to do this.

Great. If this should be done in a different PR, I don't mind to implement this using your new function, or if you can do it here, great too.

…he actual end rather than stalling before it
@LeonarddeR

Copy link
Copy Markdown
Collaborator Author

@nvdaes I think the last commit should fix this and make this consistent to other textInfo impls.

@nvdaes

nvdaes commented Nov 14, 2025

Copy link
Copy Markdown
Collaborator

Leonard wrote:

I think the last commit should fix this and make this consistent to other textInfo impls.

The behavior is not consistent with other programs, since the cursor moves to the last character of the document, not to the end, i.e., after the last character.
Locally, I fixed this like that, using your function in the compoundDocuments file:

def allowMoveToUnitOffsetPastEnd(self, unit: str) -> bool:
return unit in (textInfos.UNIT_CHARACTER, textInfos.UNIT_WORD) or self.obj.flowsTo is None

This produces a consistent behavior in LibreOffice Writer, though perhaps another solution is better.

@LeonarddeR

Copy link
Copy Markdown
Collaborator Author

@nvdaes wrote:

Locally, I fixed this like that, using your function in the compoundDocuments file:

def allowMoveToUnitOffsetPastEnd(self, unit: str) -> bool:
return unit in (textInfos.UNIT_CHARACTER, textInfos.UNIT_WORD) or self.obj.flowsTo is None

This produces a consistent behavior in LibreOffice Writer, though perhaps another solution is better.

No no, this solution is actually very elegant and perfect, thank you very much for proposing it. I added it just now.

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

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 7 out of 7 changed files in this pull request and generated 1 comment.


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

Comment thread tests/unit/test_compoundDocuments.py
@seanbudd

Copy link
Copy Markdown
Member

Thanks @LeonarddeR - this fix looks good to me but would also like a review from @SaschaCowley .
Additionally, have you had a chance to investigate system tests?

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

Thanks, @LeonarddeR, this looks good to me too

Comment thread tests/unit/objectProvider.py
@LeonarddeR

Copy link
Copy Markdown
Collaborator Author

Apart from that I'm not very comfortabele with system test writing yet, I think it is a problem that a runner might need to install LibreOffice before it can be used. A LibreOffice installation takes a significant amount of time. May be we can use the version in the portable apps format

@seanbudd

Copy link
Copy Markdown
Member

A LibreOffice installation takes a significant amount of time. May be we can use the version in the portable apps format

Yeah caching a portable format I think would be ideal

@seanbudd seanbudd enabled auto-merge (squash) November 17, 2025 06:27
@seanbudd seanbudd merged commit 125915b into nvaccess:master Nov 17, 2025
77 of 79 checks passed
@github-actions github-actions Bot added this to the 2026.1 milestone Nov 17, 2025
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.

LibreOffice Writer: Cannot move braille to next paragraph

6 participants