Skip to content

Word with UIA: Fix case where comment contents don't read#13789

Closed
LeonarddeR wants to merge 1 commit into
nvaccess:masterfrom
LeonarddeR:commentReading
Closed

Word with UIA: Fix case where comment contents don't read#13789
LeonarddeR wants to merge 1 commit into
nvaccess:masterfrom
LeonarddeR:commentReading

Conversation

@LeonarddeR
Copy link
Copy Markdown
Collaborator

@LeonarddeR LeonarddeR commented Jun 13, 2022

Link to issue number:

None

Summary of the issue:

When playing with comments in Word 16.0.15225.20204 lately, I discovered that NVDA was unable to show the comment contents in the elements list or when pressing NVDA+Alt+C

Description of how this pull request fixes the issue:

Somehow, Microsoft decided to describe the comment thread in the name property and moved the contents of the comment to the Full Description property.

Testing strategy:

Created a new document, added a comment. Checked in both the elements list and when pressing alt+NVDA+C that the comment read correctly.

Known issues with pull request:

I'm not sure whether my assumption is correct that Microsoft really changed something here, but given this seems to have worked before, I guess that this is the case. Therefore I built a workaround that should fallback to the name of the comment UIAElement if the FullDescription is empty, however I'm unable to test this for older versions of Word. it would really help if Microsoft would clarify things here.

Change log entries:

Bug fixes

  • NVDA is no longer unable to read the contents of a comment in newer versions of MS Word when UIA is used.

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

@LeonarddeR LeonarddeR requested a review from a team as a code owner June 13, 2022 18:19
@LeonarddeR
Copy link
Copy Markdown
Collaborator Author

Explicitly requesting a review from @michaelDCurran here

@seanbudd
Copy link
Copy Markdown
Member

This perhaps should target beta. Have you reported this to MS Word?

Can anyone test this PR with an older version of Word?

@cary-rowen
Copy link
Copy Markdown
Contributor

I can confirm that this PR addresses the issue of alpha-25551 not reporting comment content in 'Microsoft 365MSO (Version 2205 Build 16.0.15225.20172)'.

@seanbudd: "Can anyone test this PR with an older version of Word?"

What is the old version of mso you expect to test, I might be able to test it.

@LeonarddeR
Copy link
Copy Markdown
Collaborator Author

I tried to change the target branch, but Github gets stuck at that point with a loading message for the target branches.

@LeonarddeR
Copy link
Copy Markdown
Collaborator Author

Have you reported this to MS Word?

NO, since this is in fact not a bug. Pretty sure that in earlier version of word, you wouldn't be able to find out who was the author of a comment when walking through the comments tree. However, we need to know when this change happened so we can come up with an evidence based solution.

@seanbudd seanbudd changed the base branch from master to beta June 14, 2022 05:51
@seanbudd
Copy link
Copy Markdown
Member

I've updated the target branch.

@LeonarddeR - could you provide an estimate to @cary-rowen?

@seanbudd: "Can anyone test this PR with an older version of Word?"

What is the old version of mso you expect to test, I might be able to test it.

@LeonarddeR
Copy link
Copy Markdown
Collaborator Author

That's why I asked @michaelDCurran to review this as well😉, because he might be able to find out what version of Word he tested with when working on this lately. I think it was in februari, see #13341. So I assume a version between February and now changed this.

# Use Annotation Type Comment if available
if typeID == UIAHandler.AnnotationType_Comment:
comment = UIAElement.GetCurrentPropertyValue(UIAHandler.UIA_NamePropertyId)
# Newer versions of Word present the comment text in the FullDescription property
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
# Newer versions of Word present the comment text in the FullDescription property
# Newer versions (which? estimate?) of Word present the comment text in the FullDescription property

@seanbudd seanbudd changed the base branch from beta to master June 14, 2022 06:38
@michaelDCurran
Copy link
Copy Markdown
Member

I don't remember what version of Word I tested against, but I'm pretty sure it had already changed before then. I.e. I was already seeing info about the comment thread, but not individual comment contents.
In fact, I thought it may have happened when they introduced the new comments pain in 16.0.1493 or so.
Currently in NVDA, for a comment thread, it just describes the comment thread itself, and how many replies.
This PR on the other hand, will describe the first comment in the thread.
I guess it depends on which we think is more useful and appropriate?
Possibly describing the first comment and number of replies?
These days, to actually interact with threaded comments in MS Word, I would suggest that the user uses the Modern comments pane built in to MS Word. I.e. move to a comment, then press alt+f12 to move focus to the comment and thread of replies.

@cary-rowen
Copy link
Copy Markdown
Contributor

Not sure if this is related, there is a document with comment and I can see the content in the element dialog, but press NVDA +Alt +C and NVDA only reports 'no comment'.

test.docx
@LeonarddeR can you take a look at this?
Both the latest Alpha and the build of this PR are reproduce this.

@seanbudd seanbudd added the blocked/needs-info The issue can not be progressed until more information is provided. label Jun 17, 2022
@LeonarddeR LeonarddeR marked this pull request as draft June 18, 2022 07:31
@LeonarddeR
Copy link
Copy Markdown
Collaborator Author

Currently in NVDA, for a comment thread, it just describes the comment thread itself, and how many replies. This PR on the other hand, will describe the first comment in the thread. I guess it depends on which we think is more useful and appropriate? Possibly describing the first comment and number of replies? These days, to actually interact with threaded comments in MS Word, I would suggest that the user uses the Modern comments pane built in to MS Word. I.e. move to a comment, then press alt+f12 to move focus to the comment and thread of replies.

We at least want to show the threads in the elements list I assume. I will try to look into this furhter, but it is pretty hard to do so when I can't test with older versions. Having said that, as we stick to UIA by default in 16.0.1500 or newer, would it be safe to drop the older comments support altogether (i.e. the part that constructs a comment announcement based on scary object nav)?

@michaelDCurran
Copy link
Copy Markdown
Member

michaelDCurran commented Jun 18, 2022 via email

@LeonarddeR LeonarddeR changed the base branch from master to beta June 21, 2022 17:27
@seanbudd
Copy link
Copy Markdown
Member

seanbudd commented Jul 12, 2022

Is there a reason why this is still targeting beta?

@LeonarddeR
Copy link
Copy Markdown
Collaborator Author

I hoped it would be an easy fix, but it turns out it isn't. I have a week off in two weeks so I should be able to look into it then.

@LeonarddeR LeonarddeR added this to the 2022.3 milestone Jul 13, 2022
@LeonarddeR LeonarddeR changed the base branch from beta to master July 13, 2022 09:57
@LeonarddeR
Copy link
Copy Markdown
Collaborator Author

I thought this would be an easy fix, but for all comments to end up in the elements list in a threaded fashion, I'm, pretty sure a new iterator is required. This falls far beyond my skills I'm afraid.

@LeonarddeR LeonarddeR closed this Jul 15, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

blocked/needs-info The issue can not be progressed until more information is provided.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants