-
-
Notifications
You must be signed in to change notification settings - Fork 654
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.
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
soffice: Report keyboard-triggered formatting toggles in Writer #16413
soffice: Report keyboard-triggered formatting toggles in Writer #16413
Conversation
### Link to issue number: Fixes nvaccess#4248 ### Summary of the issue: When toggling text formatting in LibreOffice Writer using keyboard shortcuts (eg. Ctrl+B for toggling bold when using English UI), NVDA was not reporting whether bold was enabled or not. ### Description of user facing changes In LibreOffice Writer (version 24.8 and newer), when toggling text formatting (bold, italic, underline, subscript/superscript, alignment) using the corresponding keyboard shortcut, NVDA announces the new formatting attribute (e.g. "Bold on", "Bold off"), similar to how it does for Microsoft Word. ### Description of development approach In the SymphonyDocument class, register gestures for keyboard shortcuts that can be used to toggle formatting in LibreOffice Writer. Add a method to handle the gestures, which is the same for all attributes and enables processing of state change events on toolbar buttons until a timeout (of currently 0.15 seconds) is reached. During that time, when a state change event for a toolbar button is received from LibreOffice, the name of the button and its new state (on/off) are announced. This also requires a corresponding change in LibreOffice [1] so that the state change events for toolbar buttons are actually sent, contained in LibreOffice >= 24.8: commit 0425b6eb47830b1fe630dc0128d5049f4b3e5582 Author: Michael Weghorn <m.weghorn@posteo.de> Date: Tue Apr 16 19:02:30 2024 +0200 tdf#160695 wina11y: Send status change events for toolbar buttons Note: An alternative approach querying the text attributes from the IAccessibleText interface instead of the button state would run into the problem that it works reliably when text is selected, but has no clear way to figure out whether or not bold is enabled or not when there's no selection. (The text attributes need to be queried for a given index, but when the text cursor is e.g. in front of the first character of non-bold text, Ctrl+B would enable bold for newly typed text, but querying the text attributes at index 0 of the existing text would still not report bold, resulting in issues like as observed and described in issue nvaccess#16412 for MS Word.) [1] https://git.libreoffice.org/core/commit/0425b6eb47830b1fe630dc0128d5049f4b3e5582 ### Testing strategy: 1. start NVDA 2. start LibreOffice Writer (current development version, e.g. an upcoming daily build available from [2]). 3. Press Ctrl+B to enable Bold 4. Verify that NVDA announces "Bold on" 5. Press Ctrl+B again to disable bold formatting 6. Verify that NVDA announces "Bold off" 7. Type "hello world" 8. repeat steps 3-6 9. move text cursor to the beginning of the text 10. repeat steps 3-6 11. select the text 12. repeat steps 3-6 13. repeat steps 1-12 for other formatting attributes (Ctrl+I for italic, Ctrl+U for underline,...) [2] https://dev-builds.libreoffice.org/daily/master/current.html ### 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.
87251f5
to
84e546e
Compare
I am not a user of LO and will probably not perform tests. One question though. Are the toolbars or their buttons always present or is this configurable by the user (e.g. show/hide toolbar, remove button, etc.)? |
Both, the user interface and the keyboard shortcuts are highly customizable in LibreOffice, so it's e.g. possible for the user to remove buttons or switch to a different interface altogether. (The Tabbed interface has its own a11y issues at the moment though, otherwise announcement should in my understanding also work with that one as long as the corresponding buttons are shown.) In the case of removing buttons or hiding toolbars, formatting toggles are no longer announced. At first thought, this seems even somewhat in line with what sighted users experience: If the buttons are hidden, there is no more visual feedback either. An alternative approach to relying on the buttons I thought of initially was using text attributes, but that cannot reliably detect whether e.g. bold is on/off when there's no text selection, s. PR description. |
This makes sense. Just check that NVDA fails gracefully in this case or in the case Libre Office is too old, i.e. nothing reported, but no error in the log; a debugWarning may be suitable though.
Yes, same for MS Word. |
In that case, no state-changed event is emitted on LO side, so simply "nothing happens" (as the newly added |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Below are some comments on the code.
Also, did you test with quick key typing? E.g. double press ``control+bor even keep
control+b` pressed during two seconds.
# button's accessible name is the font attribute, e.g. "Bold", "Italic" | ||
if enabled: | ||
# Translators: a message when toggling formatting (e.g. bold, italic) in LibreOffice | ||
message = _("{textAttribute} on").format(textAttribute=self.name) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is self.name
translated in non-English UI of LO?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes. I just tested with German UI, and pressing Ctrl+B causes the announcement of "Blocksatz on". This is not the equivalent of "Bold", but "Justified", as the German keyboard shortcuts are different.
From what I could see, taking care that the right keyboard shortcuts/gestures are handled is generally a responsibility of the corresponding translation (i.e. language communities). Is that correct?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes until now, localization of shortcuts are handled by translators.
This may change or not change in a near future since the translation process is currently being modified.
But until NV Access gives further instructions, let's stick to the old method.
source/appModules/soffice.py
Outdated
# send gesture | ||
gesture.send() | ||
|
||
__gestures = { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The preferred method to associate gestures to scripts is the @script
decorator; could you use it? Or is there any reason why you did not?
Note that you can define the @script
decorator with only the gesture
or the gestures
parameters, i.e. you can omit description
and category
so that the script is not remappable.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, I've updated this accordingly.
(The only reason I didn't do that was that I previously looked at another example that didn't (script_toggleBold
in source/NVDAObjects/window/winword.py
) and did the same as that one, rather than starting from scratch using the NVDA developer guide where this is explained.)
As suggested in review: nvaccess#16413 (comment)
Thanks!
I hadn't tested that before, but did now. In both cases, just the last state transition is announced, which seems to be the correct behavior to me. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @michaelweghorn , just minor issues
user_docs/en/changes.t2t
Outdated
@@ -44,6 +44,7 @@ What's New in NVDA | |||
- The volume of the other applications can be adjusted by ``NVDA+alt+pageUp`` and ``NVDA+alt+pageDown``. (#16052, @mltony) | |||
- The sound of the other applications can be muted with ``NVDA+alt+delete``. (#16052, @mltony) | |||
- | |||
- In LibreOffice Writer (version 24.8 and newer), when toggling text formatting (bold, italic, underline, subscript/superscript, alignment) using the corresponding keyboard shortcut, NVDA announces the new formatting attribute (e.g. "Bold on", "Bold off"). (#4248, @michaelweghorn) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this will need to be moved to 2024.3 when the change log section is created
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there anything to do for me at the moment or just wait for now?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
just wait
source/appModules/soffice.py
Outdated
) | ||
def script_toggleTextAttribute(self, gesture): | ||
# reset time and enable announcement of toggled toolbar buttons | ||
# s. SymphonyButton#event_stateChange |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
typo here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changed to this now:
# see L{SymphonyButton.event_stateChange}
Is this the right way to reference that method? I've seen that being used elsewhere.
(From a quick search for Sphinx examples, I've found mentions of syntax using :func:<method>
, which would potentially mean
# see :func:`SymphonyButton.event_stateChange`
here? But I didn't see such syntax being used anywhere in the NVDA code base, so used what I saw elsewhere for now.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We have only just migrated to using Sphinx for new code instead of epydoc, I think using the Sphinx method is preferred here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK, switched to the Sphinx method (as I understand it) now.
s. nvaccess#16413 (review) Co-authored-by: Cyrille Bougot <cyrille.bougot2@laposte.net>
…ice_report_formatting_toggles
…cess#16413) * soffice: Report keyboard-triggered formatting toggles in Writer ### Link to issue number: Fixes nvaccess#4248 ### Summary of the issue: When toggling text formatting in LibreOffice Writer using keyboard shortcuts (eg. Ctrl+B for toggling bold when using English UI), NVDA was not reporting whether bold was enabled or not. ### Description of user facing changes In LibreOffice Writer (version 24.8 and newer), when toggling text formatting (bold, italic, underline, subscript/superscript, alignment) using the corresponding keyboard shortcut, NVDA announces the new formatting attribute (e.g. "Bold on", "Bold off"), similar to how it does for Microsoft Word. ### Description of development approach In the SymphonyDocument class, register gestures for keyboard shortcuts that can be used to toggle formatting in LibreOffice Writer. Add a method to handle the gestures, which is the same for all attributes and enables processing of state change events on toolbar buttons until a timeout (of currently 0.15 seconds) is reached. During that time, when a state change event for a toolbar button is received from LibreOffice, the name of the button and its new state (on/off) are announced. This also requires a corresponding change in LibreOffice [1] so that the state change events for toolbar buttons are actually sent, contained in LibreOffice >= 24.8: commit 0425b6eb47830b1fe630dc0128d5049f4b3e5582 Author: Michael Weghorn <m.weghorn@posteo.de> Date: Tue Apr 16 19:02:30 2024 +0200 tdf#160695 wina11y: Send status change events for toolbar buttons Note: An alternative approach querying the text attributes from the IAccessibleText interface instead of the button state would run into the problem that it works reliably when text is selected, but has no clear way to figure out whether or not bold is enabled or not when there's no selection. (The text attributes need to be queried for a given index, but when the text cursor is e.g. in front of the first character of non-bold text, Ctrl+B would enable bold for newly typed text, but querying the text attributes at index 0 of the existing text would still not report bold, resulting in issues like as observed and described in issue nvaccess#16412 for MS Word.) [1] https://git.libreoffice.org/core/commit/0425b6eb47830b1fe630dc0128d5049f4b3e5582 ### Testing strategy: 1. start NVDA 2. start LibreOffice Writer (current development version, e.g. an upcoming daily build available from [2]). 3. Press Ctrl+B to enable Bold 4. Verify that NVDA announces "Bold on" 5. Press Ctrl+B again to disable bold formatting 6. Verify that NVDA announces "Bold off" 7. Type "hello world" 8. repeat steps 3-6 9. move text cursor to the beginning of the text 10. repeat steps 3-6 11. select the text 12. repeat steps 3-6 13. repeat steps 1-12 for other formatting attributes (Ctrl+I for italic, Ctrl+U for underline,...) [2] https://dev-builds.libreoffice.org/daily/master/current.html ### 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. * Follow-up: Use @script decorator As suggested in review: nvaccess#16413 (comment) * Follow-up: Address review comments s. nvaccess#16413 (review) * Follow-up: Use function docstring instead of comments s. nvaccess#16413 (review) Co-authored-by: Cyrille Bougot <cyrille.bougot2@laposte.net> * Follow-up: Use Sphinx syntax for reference s. nvaccess#16413 (comment) --------- Co-authored-by: Cyrille Bougot <cyrille.bougot2@laposte.net> Co-authored-by: Sean Budd <sean@nvaccess.org>
These shortcuts need to be translated by translators for non-English GUIs. In French for example, it seems that all shortcuts are the same except for bold which is control+G instead of control+B. @michaelweghorn, I have tested with a portable version of LO < 24.8 (maybe 24.2.4). Strangely, it was already working when toggling "underline" but not with the other ones. Any explanation? More generally, would you be able to point me in LO's code where the localized formatting shortcuts are defined. This may be a helpful information to pass to translators. |
@CyrilleB79: Thanks for looking into this.
Before LibreOffice commit https://git.libreoffice.org/core/commit/0425b6eb47830b1fe630dc0128d5049f4b3e5582 , toolbar items that have a "toggle button" role in LO wouldn't send the necessary event. However, the "underline" control in the formatting toolbar has a "dropdown button" role, which is presumably why it was unaffected and already working fine.
They're defined in this XML file: https://git.libreoffice.org/core/+/refs/heads/master/officecfg/registry/data/org/openoffice/Office/Accelerators.xcu For the Ctrl+G example in French you mention, that would be the this entry:
For German, bold (
where The relevant so-called "UNO commands" to look for are:
If there's a language-specific entry, like |
Thanks for the details. I'll send make the chenges for French and send a message on the translators mailing list. |
### Link to issue number: Partially implements feature requests from nvaccess#6915 ### Summary of the issue: commit 46a3436 ("soffice: Report keyboard-triggered formatting toggles in Writer (nvaccess#16413)") implemented announcement of formatting changes triggered by keyboard shortcuts for formatting attributes whose state is represented by toggle buttons in Writer's formatting toolbar. Issue nvaccess#6915 requests announcement of further text formatting attributes that are not represented by simple toggle buttons. This includes decreasing/increasing the font size, which can be done using the Ctrl+[ and Ctrl+] shortcuts when using an English (US) UI and keyboard layout. ### Description of user facing changes When decreasing or increasing the font size in LibreOffice Writer using the corresponding keyboard shortcuts, NVDA announces the new font size. ### Description of development approach Extend the solution from the above-mentioned commit 46a3436 to not only cover toggle buttons, but also UI controls implementing the IAccessibleText interface and handle the gestures/keyboard shortcuts for changing the font size: * Rename methods and variables introduced earlier to not have button-specific names. * Extract some logic to helper methods/classes to avoid duplication in `SymphonyButton` and the newly introduced `SymphonyText` logic. * Add Ctrl+[ and Ctrl+] to the list of keyboard gestures to handle. * Add `SymphonyText.event_valueChange` override that announces the new value. This gets triggered when the value of the "Font Size" editable combobox in Writer's formatting toolbar changes after using the keyboard shortcut, and results in the new value being announced, e.g. "14 pt". This is comparable to the handling in `SymphonyButton.event_stateChange` introduced in the earlier above-mentioned commit. * Increase the timeout for announcement of events from 0.15 to 2 seconds, as 0.15 seconds wasn't always sufficient when testing the new feature. ### Testing strategy: 1. start NVDA 2. start LibreOffice Writer (with English UI and keyboard layout) 3. Press Ctrl+[ to decrease the font size 4. Verify that NVDA announces the new font size, e.g. "10 pt" when using the default document template 5. Press Ctrl+] to increase the font size 4. Verify that NVDA announces the new font size, e.g. "12 pt" when using the default document template 5. repeat the tests mentioned in commit 46a3436 to verify the toggle button case still works as expected ### 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.
While issue nvaccess#16413 also requested announcement of formatting changes, PR nvaccess#17147 is for issue nvaccess#6915.
Partially implements feature requests from #6915 Summary of the issue: commit 46a3436 ("soffice: Report keyboard-triggered formatting toggles in Writer (#16413)") implemented announcement of formatting changes triggered by keyboard shortcuts for formatting attributes whose state is represented by toggle buttons in Writer's formatting toolbar. Issue #6915 requests announcement of further text formatting attributes that are not represented by simple toggle buttons. This includes decreasing/increasing the font size, which can be done using the Ctrl+[ and Ctrl+] shortcuts when using an English (US) UI and keyboard layout. Description of user facing changes When decreasing or increasing the font size in LibreOffice Writer using the corresponding keyboard shortcuts, NVDA announces the new font size. Description of development approach Extend the solution from the above-mentioned commit 46a3436 to not only cover toggle buttons, but also UI controls implementing the IAccessibleText interface and handle the gestures/keyboard shortcuts for changing the font size: Rename methods and variables introduced earlier to not have button-specific names. Extract some logic to helper methods/classes to avoid duplication in SymphonyButton and the newly introduced SymphonyText logic. Add Ctrl+[ and Ctrl+] to the list of keyboard gestures to handle. Add SymphonyText.event_valueChange override that announces the new value. This gets triggered when the value of the "Font Size" editable combobox in Writer's formatting toolbar changes after using the keyboard shortcut, and results in the new value being announced, e.g. "14 pt". This is comparable to the handling in SymphonyButton.event_stateChange introduced in the earlier above-mentioned commit. Increase the timeout for announcement of events from 0.15 to 2 seconds, as 0.15 seconds wasn't always sufficient when testing the new feature.
### Link to issue number: Implements one aspect from feature requests in nvaccess#6915 ### Summary of the issue: Commit 46a3436 ("soffice: Report keyboard-triggered formatting toggles in Writer (nvaccess#16413)") implemented announcement of keyboard-triggered formatting changes in LibreOffice Writer, including the one to toggle underline. (When English keyboard layout and UI, the shortcut is Ctrl+B.) Issue nvaccess#6915 (among others) requests announcement also for "toggle double underline", which is done via the Ctrl+D keyboard shortcut for an English UI. ### Description of user facing changes When toggling double underline in LibreOffice Writer using the corresponding keyboard shortcut, NVDA announces the new state ("double underline on"/"double underline off"). (nvaccess#6915, @michaelweghorn) ### Description of development approach Add Ctrl+D to the list gestures handled by the existing gesture handler. ### Testing strategy: 1. start NVDA 2. start LibreOffice Writer (with English UI and keyboard layout) 3. Press Ctrl+D to enable double underline 4. Verify that NVDA announces "double underline on" 5. Press Ctrl+D to disable double underline again 6. Verify that NVDA announces "double underline off" 7. Press Ctrl+U to enable (normal/single) underline 8. Verify that NVDA announces "underline on" 9. Press Ctrl+D to enable double underline 10. Verify that NVDA announces "double underline on" ### 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. <!-- Please keep the following --> @coderabbitai summary
) Implements one aspect from feature requests in #6915 Summary of the issue: Commit 46a3436 ("soffice: Report keyboard-triggered formatting toggles in Writer (#16413)") implemented announcement of keyboard-triggered formatting changes in LibreOffice Writer, including the one to toggle underline. (When English keyboard layout and UI, the shortcut is Ctrl+B.) Issue #6915 (among others) requests announcement also for "toggle double underline", which is done via the Ctrl+D keyboard shortcut for an English UI. Description of user facing changes When toggling double underline in LibreOffice Writer using the corresponding keyboard shortcut, NVDA announces the new state ("double underline on"/"double underline off"). (#6915, @michaelweghorn) Description of development approach Add Ctrl+D to the list gestures handled by the existing gesture handler.
Link to issue number:
Fixes #4248
Summary of the issue:
When toggling text formatting in LibreOffice Writer using keyboard shortcuts (eg. Ctrl+B for toggling bold when using English UI), NVDA was not reporting whether bold was enabled or not.
Description of user facing changes
In LibreOffice Writer (version 24.8 and newer), when toggling text formatting (bold, italic, underline, subscript/superscript, alignment) using the corresponding keyboard shortcut, NVDA announces the new formatting attribute (e.g. "Bold on", "Bold off"), similar to how it does for Microsoft Word.
Description of development approach
In the SymphonyDocument class,
register gestures for keyboard shortcuts that
can be used to toggle formatting in LibreOffice Writer.
Add a method to handle the gestures, which is the same for all attributes and enables processing of state change events on toolbar buttons until a timeout
(of currently 0.15 seconds) is reached.
During that time, when a state change event for
a toolbar button is received from LibreOffice,
the name of the button and its new state (on/off)
are announced.
This also requires a corresponding change in LibreOffice [1] so that the state change events for toolbar buttons are actually sent, contained in LibreOffice >= 24.8:
Note: An alternative approach querying the text attributes from the IAccessibleText interface instead of the button state would run into the problem that it works reliably when text is selected, but has no clear way to figure out whether or not bold is enabled or not when there's no selection. (The text attributes need to be queried for a given index, but when the text cursor is e.g. in front of the first character of non-bold text, Ctrl+B would enable bold for newly typed text, but querying the text attributes at index 0 of the existing text would still not report bold, resulting in issues like as observed and described in issue #16412 for MS Word.)
[1] https://git.libreoffice.org/core/commit/0425b6eb47830b1fe630dc0128d5049f4b3e5582
Testing strategy:
[2] https://dev-builds.libreoffice.org/daily/master/current.html
Known issues with pull request:
None
Code Review Checklist: