Skip to content
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

Reporting indent in Word with UIA (fix-up) #12924

Merged
merged 4 commits into from
Oct 18, 2021

Conversation

CyrilleB79
Copy link
Collaborator

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:

  • UIA first line indent corresponds to Word's first line indent
  • UIA leading indent corresponds to Word's left indent
  • UIA trailing indent corresponds to Word's right indent
    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:
  • left indent = 2.5cm and first line indent = 2.5cm
  • left indent = 2.5cm and first line negative indent = 2.5cm

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

  • left indent = distance from the left margin to the left-most start of a line (it may be the first line or the following lines)
  • right indent = distance from the right margin to the end of the paragraph's lines (except the last that may be incomplete)
  • first line indent = distance between the start of the first line and the start of the other lines when the first line starts after the other ones
  • hanging indent = distance between the start of the first line and the start of the other lines when the first line starts before the other ones

UIA terminology

  • first line indent = distance from the left margin to the start of the first line of the paragraph
  • leading indent: distance from the left margin to the start of the following lines (2nd, 3rd, etc.)
  • trailing indent = distance from the right margin to the end of the paragraph's lines (except the last that may be incomplete)

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:

  • no first line indent
  • first line indent > 0
  • negative indent > 0

Known issues with pull request:

Code design:
The getIndentValueDisplayString function stands alone in the NVDAObjects\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:

  • Pull Request description:
    • description is up to date
    • change log entries
  • Testing:
    • Unit tests
    • System (end to end) tests
    • Manual testing
  • API is compatible with existing add-ons.
  • Documentation:
    • User Documentation
    • Developer / Technical Documentation
    • Context sensitive help for GUI changes
  • UX of all users considered:
    • Speech
    • Braille
    • Low Vision
    • Different web browsers
    • Localization in other languages / culture than English

- fixed issue when first line indent is negative
- do not report 0 values in formatting information
@CyrilleB79
Copy link
Collaborator Author

Cc @michaelDCurran

And while at it, would you mind give your opinion on #12908 (comment)?

@CyrilleB79 CyrilleB79 marked this pull request as ready for review October 11, 2021 22:31
@CyrilleB79 CyrilleB79 requested a review from a team as a code owner October 11, 2021 22:31
Also _getIndentValueDisplayString moved in the class and made private.
Copy link
Member

@seanbudd seanbudd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @CyrilleB79, I've added some minor notes to the documentation but this looks ready to be merged otherwise.

source/NVDAObjects/UIA/__init__.py Outdated Show resolved Hide resolved
source/NVDAObjects/UIA/__init__.py Outdated Show resolved Hide resolved
source/NVDAObjects/UIA/__init__.py Outdated Show resolved Hide resolved
source/NVDAObjects/UIA/__init__.py Outdated Show resolved Hide resolved
source/NVDAObjects/UIA/__init__.py Outdated Show resolved Hide resolved
source/NVDAObjects/UIA/__init__.py Outdated Show resolved Hide resolved
Co-authored-by: Sean Budd <seanbudd123@gmail.com>
@AppVeyorBot
Copy link

See test results for failed build of commit 190d4a3379

@CyrilleB79
Copy link
Collaborator Author

@seanbudd I have addressed all the review comments.
I have had to remove the star import to content the linter.
To know with which variable I should replace the import *, I have:

  1. Run the linter on the whole file (not only the diff)
  2. Searched the result for errors relative to undeclared variables / start import to identify the missing explicit declarations.
  3. Checked in UIAUtils.py that the definitions of all these variables were actually present in this file.

Copy link
Member

@seanbudd seanbudd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @CyrilleB79

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.

Enabling UIA Support in Word Causes Paragraph Indentation Not to be Reported
5 participants