Skip to content

soffice: Announce actually deleted text on Ctrl+backspace#15558

Merged
seanbudd merged 1 commit into
nvaccess:masterfrom
michaelweghorn:michaelweghorn/soffice_announce_text_deleted_by_backspace
Oct 5, 2023
Merged

soffice: Announce actually deleted text on Ctrl+backspace#15558
seanbudd merged 1 commit into
nvaccess:masterfrom
michaelweghorn:michaelweghorn/soffice_announce_text_deleted_by_backspace

Conversation

@michaelweghorn

Copy link
Copy Markdown
Contributor

Link to issue number:

Fixes #15436

Summary of the issue:

The base class implementation
EditableText#_backspaceScriptHelper for handling the Ctrl+Backspace keyboard shortcut retrieves the text to announce as deleted by expanding the text info to the corresponding text unit and then retrieving the text. This gives an incorrect result in at least Libreoffice Writer, where that would return a string just containing the space character when removing a word followed by a space using the Ctrl+backspace keyboard shortcut.
(Writer's IAccessibleText::textAtOffset implementation that gets used in IA2TextTextInfo#_getWordOffsets handles the space as a separate word, but Ctrl+backspace removes both, the actual word and following whitespace.)

Description of user facing changes

When removing a word followed by whitespace in Libreoffice Writer, the actually removed text is announced.

Description of development approach

Instead of separately retrieving the text for a word, override the default implementation in the LibreOffice app module and announce the actually removed text when handling Ctrl+backspace.
Use the caret position before and after the text has been removed to identify what the removed text is.

Testing strategy:

Test the scenario as described in issue #15436:

  1. start LibreOffice Writer
  2. type "NVDA is a free screen reader."
  3. press Ctrl+backspace 7 times
  4. verify that the actually removed text is annonced by NVDA.

Known issues with pull request:

None

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.

@AppVeyorBot

Copy link
Copy Markdown

See test results for failed build of commit 80c8fefaf3

@OzancanKaratas

Copy link
Copy Markdown
Collaborator

Please do not touch the changes file. This file will be changed by NV Access.

@michaelweghorn

Copy link
Copy Markdown
Contributor Author

Please do not touch the changes file. This file will be changed by NV Access.

There was this recent email on the nvda-devel mailing list about a change on how this is handled:
https://groups.io/g/nvda-devel/message/46876

My understanding is that the changes file should be modified directly now (which this change does), or do I misunderstand anything?

@OzancanKaratas

Copy link
Copy Markdown
Collaborator

No. Do not revert. I misled you because I was not aware of the final decision. I am sorry.

@michaelweghorn

Copy link
Copy Markdown
Contributor Author

No problem, thanks for taking a look.

@seanbudd seanbudd added the conceptApproved Similar 'triaged' for issues, PR accepted in theory, implementation needs review. label Oct 2, 2023

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

Comment thread source/appModules/soffice.py Outdated
Comment thread source/appModules/soffice.py Outdated
Comment thread user_docs/en/changes.t2t Outdated
@michaelweghorn michaelweghorn force-pushed the michaelweghorn/soffice_announce_text_deleted_by_backspace branch from 401a0f2 to 7b0a2f6 Compare October 3, 2023 07:03
@AppVeyorBot

Copy link
Copy Markdown

See test results for failed build of commit 95627e81a1

Comment thread source/appModules/soffice.py Outdated
@michaelweghorn michaelweghorn force-pushed the michaelweghorn/soffice_announce_text_deleted_by_backspace branch from 7b0a2f6 to 65bf1aa Compare October 4, 2023 08:03
Comment thread user_docs/en/changes.t2t Outdated
@michaelweghorn michaelweghorn force-pushed the michaelweghorn/soffice_announce_text_deleted_by_backspace branch from 65bf1aa to 5c83fad Compare October 5, 2023 06:13
 ### Link to issue number:

Fixes nvaccess#15436

 ### Summary of the issue:

The base class implementation
`EditableText#_backspaceScriptHelper` for handling
the Ctrl+Backspace keyboard shortcut retrieves the text to
announce as deleted by expanding the text info to the
corresponding text unit and then retrieving the text.
This gives an incorrect result in at least Libreoffice Writer,
where that would return a string just containing the space character
when removing a word followed by a space using the
Ctrl+backspace keyboard shortcut.
(Writer's `IAccessibleText::textAtOffset` implementation
that gets used in `IA2TextTextInfo#_getWordOffsets`
handles the space as a separate word, but Ctrl+backspace
removes both, the actual word and following whitespace.)

 ### Description of user facing changes

When removing a word followed by whitespace in Libreoffice Writer,
the actually removed text is announced.

 ### Description of development approach

Instead of separately retrieving the text for a word,
override the default implementation in the LibreOffice app
module and announce the actually removed text when handling
Ctrl+backspace.
Use the caret position before and after the text has
been removed to identify what the removed text is.

 ### Testing strategy:

Test the scenario as described in issue nvaccess#15436:

1) start LibreOffice Writer
2) type "NVDA is a free screen reader."
3) press Ctrl+backspace 7 times
4) verify that the actually removed text is annonced by NVDA.

 ### Known issues with pull request:

None

 ### Code Review Checklist:

- [x] Documentation:
  - Change log entry
  - User Documentation
  - Developer / Technical Documentation
  - Context sensitive help for GUI changes
- [x] Testing:
  - Unit tests
  - System (end to end) tests
  - Manual testing
- [x] UX of all users considered:
  - Speech
  - Braille
  - Low Vision
  - Different web browsers
  - Localization in other languages / culture than English
- [x] API is compatible with existing add-ons.
- [x] Security precautions taken.
@michaelweghorn michaelweghorn force-pushed the michaelweghorn/soffice_announce_text_deleted_by_backspace branch from 5c83fad to af7f289 Compare October 5, 2023 06:15
@michaelweghorn

Copy link
Copy Markdown
Contributor Author

I did a rebase onto current git master to fix the merge conflict in the changelog.

@AppVeyorBot

Copy link
Copy Markdown

See test results for failed build of commit c3e54e03c6

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

@seanbudd seanbudd merged commit a380b6a into nvaccess:master Oct 5, 2023
@nvaccessAuto nvaccessAuto added this to the 2024.1 milestone Oct 5, 2023
@michaelweghorn michaelweghorn deleted the michaelweghorn/soffice_announce_text_deleted_by_backspace branch October 5, 2023 18:39
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.

The ctrl-backspace command doesn't read the deleted word in LibreOffice Writer

6 participants