soffice: Report page-relative caret position when available#14727
Merged
Conversation
Contributor
Author
|
If there are any suggestions on different approaches or adapting the attributes reported by LibreOffice, I'm happy for any feedback on that and won't merge the corresponding LibreOffice change ( https://gerrit.libreoffice.org/c/core/+/149051 ) before there is an agreed way on how to expose/retrieve the required information. |
See test results for failed build of commit 50f7ebf6fe |
c46af0d to
6fa49c6
Compare
michaelDCurran
requested changes
Mar 20, 2023
michaelDCurran
left a comment
Member
There was a problem hiding this comment.
This looks good, accept that I don't believe you need the __init__ methods on SymphonyDocument or SymphonyDocumentTextInfo. They don't seem to do anything, and Python will automatically call super's constructor if they are not defined.
Fixes nvaccess#11696 Related LibreOffice bug report: https://bugs.documentfoundation.org/show_bug.cgi?id=136760 Summary of the issue: When reporting the review cursor location in LibreOffice Writer, the announced pixel-related coordinates are not particularly useful to determine where in the document/page the cursor is currently located. Description of how this pull request fixes the issue: Similar to how it is done for Microsoft Word, report the current cursor/caret location relative to the current page when that information is available. To retrieve that information, this introduces and makes use of a new `SymphonyDocumentTextInfo` class that retrieves the page-relative cursor position via accessible attributes from the document object. These attributes are added to LibreOffice in this corresponding LibreOffice change: https://gerrit.libreoffice.org/c/core/+/149051 The values returned by LibreOffice Writer are in twips, so convert that to inches or centimetres, depdending on whether or not imperial measurement units are expected. If the attributes are not available, fall back to the handling that was already used so far. Testing strategy: * use a LibreOffice build that includes the changes from https://gerrit.libreoffice.org/c/core/+/149051 * start LibreOffice Writer * trigger reporting the review cursor location by pressing NVDA+shift+numpadDelete * verify that the current caret position is announced in centimetres or inches from the left and top edge of the page
6fa49c6 to
34a3db6
Compare
Contributor
Author
Indeed, I've dropped them now. Thanks! |
Contributor
Author
@michaelDCurran I pressed the "Re-request review" (or similarly labeled) button next to your username now that the requested change has been implemented. This resulted in the above action, but I didn't intentionally want to remove any reviewers. Can you give me any hint on what's the proper process? |
michaelDCurran
approved these changes
Mar 20, 2023
…_better_position_reporting
tdf-gerrit
pushed a commit
to LibreOffice/core
that referenced
this pull request
Mar 20, 2023
This introduce 2 new extended accessible attributes "cursor-position-in-page-horizontal" and "cursor-position-in-page-vertical" to expose the page-relative position of the cursor in Writer to assistive technology. This is similar to how the current page number is already exposed (attribute "page-number"). Together with a corresponding pull request for the NVDA screen reader [1], this allows NVDA to announce the position similar to how it is done for Microsoft Word (where the information is not retrieved via the accessibility APIs but the MS Office COM API, s. discussion in the corresponding NVDA issue [2] for more details). (Side note: Currently there is no a11y object for a Writer page in the a11y tree, but "normal" paragraphs are direct children of the document object.) For performance reasons, introduce a new method `SwCursorShell::GetCursorPagePos` to get the position instead of passing the result from `SwCursorShell::GetCursorDocPos` to `SwFEShell::GetRelativePagePosition` to avoid iterating over the doc pages. [1] nvaccess/nvda#14727 [2] nvaccess/nvda#11696 Change-Id: I6fd56c5c7d051840bab836ce5fe525fdc061b376 Reviewed-on: https://gerrit.libreoffice.org/c/core/+/149051 Tested-by: Jenkins Reviewed-by: Michael Weghorn <m.weghorn@posteo.de>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Link to issue number:
Fixes #11696
Related LibreOffice bug report:
https://bugs.documentfoundation.org/show_bug.cgi?id=136760
Summary of the issue:
When reporting the review cursor location in LibreOffice Writer, the announced position was not particularly useful to determine where in the document/page the cursor is currently located.
Description of user facing changes
Similar to how it is done for Microsoft Word, report the current cursor/caret location relative to the current page when that information is available for LibreOffice Writer.
Description of development approach
To retrieve the page-relative cursor/caret location, this introduces and makes use of a new
SymphonyDocumentTextInfoclass that retrieves the page-relative cursor position via accessible attributes from the document object.These attributes are added to LibreOffice in this corresponding LibreOffice change:
https://gerrit.libreoffice.org/c/core/+/149051
The values returned by LibreOffice Writer are in twips, so convert that to inches or centimetres, depending on whether or not imperial measurement units are expected.
If the attributes are not available, fall back to the handling that was already used so far.
Testing strategy:
Testing strategy:
Known issues with pull request:
none
Change log entries:
Changes
When reporting the review cursor location, the current cursor/caret location is now reported relative to the current page in LibreOffice Writer for LibreOffice versions >= 7.6, similar to what is done for Microsoft Word. (#11696)Code Review Checklist: