Reporting indent in Word with UIA (fix-up)#12924
Merged
Merged
Conversation
- fixed issue when first line indent is negative - do not report 0 values in formatting information
Contributor
Author
|
And while at it, would you mind give your opinion on #12908 (comment)? |
LeonarddeR
reviewed
Oct 12, 2021
seanbudd
reviewed
Oct 14, 2021
Also _getIndentValueDisplayString moved in the class and made private.
seanbudd
requested changes
Oct 14, 2021
seanbudd
left a comment
Member
There was a problem hiding this comment.
Thanks @CyrilleB79, I've added some minor notes to the documentation but this looks ready to be merged otherwise.
Co-authored-by: Sean Budd <seanbudd123@gmail.com>
See test results for failed build of commit 190d4a3379 |
Contributor
Author
|
@seanbudd I have addressed all the review comments.
|
15 tasks
6 tasks
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
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 #12899
Fix-up of #12908
Relates to #12770
Summary of the issue:
Historically, the indent in NVDA is reported via object model and its terminology is similar to what is found in Word's paragraph formatting dialog.
In #12908, reporting indant via UIA has been introduced. A mapping between UIA text attributes relative to indent and the type of indent that is reported was used. This mapping was doing the following assumption:
This assumption is not always correct for the first two points. And you can seen that object model access. And UIA access give not the same results in the following examples:
Moreover, first line negative was reported in the object model code, whereas it does not appear at all in the UIA code, what evidences missing code.
Description of how this pull request fixes the issue:
Correct matching between Word's and UIA terminology has been implemented. To clarify:
Word's terminology
UIA terminology
Addition: do not report zero indent
In addition, I have removed reporting any type of indent when its value is zero. Indeed, it was not reported in this case with the object model access, and I felt it was worth continuing in this way. Moreover, in #12908, nothing has been indicated that zero values should now be reported, so I assumed that it was unintentional.
Possible alternative design
I have aligned what is reported to what preexisted with the object model access, since in #12908 no information was indicating that it should have been modified.
But another option may have been to report indentation as provided by UIA and to align the object model access code to what is provided by UIA.
I have also not chosen this alternative design because I think that today, only Word provides indentation information via UIA. Thus, this makes sense to follow with Words conventions. If another provider should provide indent information one day, maybe the way indentation is reported should be reconsidered and uniformed.
Should you however prefer this alternative design, just let me know.
Testing strategy:
Tested various paragraph formatting with non-zero left indent and the 3 following cases:
Known issues with pull request:
Code design:
The
getIndentValueDisplayStringfunction stands alone in theNVDAObjects\UIA\__init__.py. If you have any better proposal, just let me know.Change log entries:
Not needed, it is just a fix-up PR.
Should the alternative design be implemented however, then the new way to report indent should then be notified.
Code Review Checklist: