Skip to content

Report background and highlighted text in MS Word #14610

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

Merged
merged 18 commits into from
Apr 4, 2023

Conversation

ABuffEr
Copy link
Contributor

@ABuffEr ABuffEr commented Feb 3, 2023

Link to issue number:

Fixes #5866, fixes #7396, fixes #12101

Summary of the issue:

In these years, Microsoft Word users working without UIA had no feedback about background and highlighting colors, often used by colleagues, teachers and students.

UIA has provided a partial solution when available, but its support for these features is far from perfect (see later).

Description of user facing changes

Assuming UIA is not available/forcibly enabled:

  • when color reporting is enabled in formatting preferences, users will hear background colors too;
  • when marked/highlighted text reporting is enabled in formatting preferences, users will hear highlighting colors ("highlighted in {color}" and "no highlighting"), regardless of color reporting.

Description of development approach

It was very difficult to find a way, so I'll add some details.

Initially, I used VBA objects approach, querying Paragraph.Shading.BackgroundPatternColor and Range.HighlightColorIndex over a winwordDocument.Range to calculate each time, so it was very slow.

Then I found this document and, after having understood what I needed and how to find the required wdDISPID_ constants, I was able to move all work on the C++ component of nvdaHelperRemote related to winword, that appears very much faster.

In window/winword.py, to take advantage of winwordColorToNVDAColor work, I added also a dict that collects indexes from wdColorIndex and decimal values from wdColor.

About speech, I had to introduce a new format field, highlight-color.

Now, UIA: sure, it already covers background and highlight colors, but expose them as the same attribute, via UIA_BackgroundColorAttributeId (and no, I asked, even visually they are distinct). There is an AnnotationType_Highlighted identifier, but it seems to not be used in this scenario.

Locally I have a possible workaround, based on the fact that, as you see in Word Home menu, highlighting color is applied to characters, background color to paragraphs; so, if textRange has a background-color, I compare it to that retrieved from paragraph and, if I receive IUnknown/MixedAttributeValue, I assume the initial one is an highlighting color and try to get the real background color from "\r" EOL.

But I'm not totally convinced, so I not included it in this PR for now.

Testing strategy:

See #5866 str and attachment.

Unfortunately, I have access only to a working copy of Word for Microsoft 365 MSO (Version 2302 Build 16.0.16124.20000), 64 bit.

I tried to avoid any dangerous assumption in C++, according to existing code, but a lot of testing is required, maybe.

Known issues with pull request:

Apparently, highlighting is not correctly reported if it's not applied on whole word (investigating).

Highlighting announcements could be too verbose, I considered to use "marked in {color}" and "not marked", but then leaved as is for clarity against Word feature. Suggestions are welcome.

Even the fact highlighted (colored) text is not reported when color reporting is enabled could be confusing for some users, so, again, a discussion is welcome.

Change log entries:

New features
Changes
Bug fixes

For Developers

  • new highlight-color format field.

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
  • Security precautions taken.

@Adriani90
Copy link
Collaborator

Wow this is great work, thank you for taking this up I was trying to find a solution for this for a very long time. This will improve the way blind and sighted people work together in a document alot.

Is there also a possibility to implement a key stroke for the browse mode to jump between highlighted text?

@cary-rowen
Copy link
Contributor

Thanks, great work.
Yep, it would be nice to have a key to jump between "highlights".

@ABuffEr
Copy link
Contributor Author

ABuffEr commented Feb 4, 2023

Hi,
first, thanks for appreciation :)
About quick nav, I think it's possible, but I don't know how much it's fast. I'm studying the relative code (a lot of small classes).
Have you concerns about the aspects I mentioned under "Known issues"?

@CyrilleB79
Copy link
Collaborator

Regarding the key to jump between highlight, that's a completely valid request. But it would better to handle this in a subsequent PR. So please, open an issue (if not yet existing) or comment in existing issue.

@LeonarddeR
Copy link
Collaborator

Might be related to #9527

@ABuffEr
Copy link
Contributor Author

ABuffEr commented Feb 7, 2023

Thanks, I'll comment there about quick nav and a new formatting helper (before to elaborate this PR, I worked on a new, related add-on).

@ABuffEr ABuffEr marked this pull request as ready for review February 24, 2023 15:38
@ABuffEr ABuffEr requested a review from a team as a code owner February 24, 2023 15:38
@ABuffEr ABuffEr requested a review from seanbudd February 24, 2023 15:38
@ABuffEr
Copy link
Contributor Author

ABuffEr commented Feb 24, 2023

Hi,
I initially declared PR as a draft, as suggested in PR template, but the committed parts are ready since I opened it.
If covering UIA is preferred, I can commit also that part, even if it's more a workaround than a solution.

@seanbudd seanbudd marked this pull request as draft February 27, 2023 22:57
@ABuffEr ABuffEr requested a review from seanbudd March 1, 2023 21:55
@seanbudd
Copy link
Member

Apologies, is this ready for review? This was marked as a draft.

@seanbudd seanbudd marked this pull request as ready for review March 29, 2023 05:37
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.

This generally looks good to me.

ABuffEr and others added 2 commits March 29, 2023 13:11
@ABuffEr
Copy link
Contributor Author

ABuffEr commented Mar 29, 2023

Hi @seanbudd,
with limitations I wrote in PR description (that I suggest to address separately), everything is ok for me.

seanbudd pushed a commit that referenced this pull request Mar 30, 2023
See #14610 (comment),

Summary of the issue:
Sometimes, people open PRs as a draft as required but do not seem to know that they need to transition it to "Ready for review" so that someone at NVAccess look at it.

In the PR template we can read:

Please initially open PRs as a draft. See https://github.com/nvaccess/nvda/wiki/Contributing

But nothing on when it should be set "Ready for review".
@seanbudd seanbudd changed the title winwordHighlightAndBackgroundColors Report background and highlighted text in MS Word Apr 4, 2023
@seanbudd seanbudd merged commit d9c0393 into nvaccess:master Apr 4, 2023
@nvaccessAuto nvaccessAuto added this to the 2023.2 milestone Apr 4, 2023
seanbudd pushed a commit that referenced this pull request Jun 30, 2023
Fixes #15075

Summary of the issue:
Highlight has not been reported when querying formatting manually via script in globalCommands, particularly useful after pr #14610.

Description of user facing changes
If a text is highlighted in Word, now it's reported with NVDA+f, even if highlight reporting is disabled in document formatting settings.

Description of development approach
Simply added "reportHighlight" as config key to query in _reportFormattingHelper, inside globalCommans.py.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
7 participants