Report comments in MS Word with UIA enabled by jakubl7545 · Pull Request #9631 · nvaccess/nvda

jakubl7545 · 2019-05-28T13:22:37Z

Link to issue number:

Summary of the issue:

With UIA enabled it is impossible to read comments. This is because comments ar filtered by its name which appeared to be different across languages.
Also when there are more than 13 comments dates can't be retrieved because don't fit on the screen.

Description of how this pull request fixes the issue:

Instead of name comments are filtered by its role and AnnotationsPattern.
In situations when date is not on the screen only comment itself and author are reported.

Testing performed:

Tested with NVDA running from source with word document which has comments like the attached one.

Known issues with pull request:

None

Change log entry:

Section: Bug fixes
It is now possible to read comments in MS Word with UIA enabled.
test.docx

zstanecic · 2019-05-28T13:44:46Z

Before merging this, can you provide the try build?

lukaszgo1 · 2019-05-28T13:50:15Z

@zstanecic the build from this PR is here
https://ci.appveyor.com/api/buildjobs/h0xaocd73ynxwnpa/artifacts/output%2Fnvda_snapshot_pr9631-17375%2C76080910.exe

LeonarddeR

I haven't tested this myself, but here are some comments based on just reading the code.

LeonarddeR · 2019-05-28T18:19:42Z

 		UIAElement=UIAElement.buildUpdatedCache(UIAHandler.handler.baseCacheRequest)
 		obj=UIA(UIAElement=UIAElement)
-		if not obj.parent or obj.parent.name!='Comment':
+		if (obj.parent.role !=controlTypes.ROLE_GROUPING and not


Why did you remove the ``if not obj.parent` here? This breaks if the object has no parent.

I focused on replacing obj.name and missed it. Thanks for pointing this.

LeonarddeR · 2019-05-28T18:22:27Z

-		if not obj.parent or obj.parent.name!='Comment':
+		if (obj.parent.role !=controlTypes.ROLE_GROUPING and not
+		obj.parent.UIAElement.getCurrentPropertyValue(UIAHandler.UIA_IsAnnotationPatternAvailablePropertyId)):
 			continue


I think it would be good to have some documentation about how you came up with the if clause above.

First I wanted to replace obj.name with something which doesn't change and decided for role which is grouping. In case it would be not enough I checked developer info for that object and discovered that comments have AnnotationPattern which may be significant for them. Then I found UIAElement.getCurrentPropertyValue checks for its presence. It requires one parameter which is propertyId. For AnnotationPattern it is 30118 but I didn't want to use that as itself it means nothing. Thats why as a parameter I used UIAHandler.UIA_IsAnnotationPatternAvailablePropertyId which provides that Id.
Thats how the process of creating looked like but I don't know how much of it should be documented. Maybe you have some ideas.

I think it is ok as it is. Thanks for providing the details here. Leaving this treat unresolved until @michaelDCurran or someone else has looked at it.

LeonarddeR · 2019-05-28T18:30:52Z

 		comment=obj.makeTextInfo(textInfos.POSITION_ALL).text
 		dateObj=obj.previous
 		date=dateObj.name
+		if not dateObj.previous:


I think that it is more readable if you check obj.previous.previous first. Now, you are assigning an object to the dateObj variable and then, when it doesn't seem to be a dateObj, you're assigning the same object to authorObj.

For example:

comment=obj.makeTextInfo(textInfos.POSITION_ALL).text tempObj = obj.previous.previous authorObj = tempObj or obj.previous author =authorObj.name if not tempObj: return dict(comment=comment,author=author) dateObj=obj.previous date=dateObj.name return dict(comment=comment,author=author,date=date)

LeonarddeR · 2019-05-28T18:31:53Z

 	@property
 	def label(self):
 		commentInfo=getCommentInfoFromPosition(self.textInfo)
+		if len(commentInfo) ==2:


Suggested change

if len(commentInfo) ==2:

if "date" not in commentInfo:

LeonarddeR · 2019-05-28T18:33:30Z

 	# Microsoft Word duplicates the full title of the document on this control, which is redundant as it appears in the title of the app itself.
 	name=u""

+	@script(gesture ="kb:NVDA+alt+c")


This lacks the description. Please reuse the description for the script for the object model implementation of Word.

Suggested change

@script(gesture ="kb:NVDA+alt+c")

@script(

gesture="kb:NVDA+alt+c",

# Translators: a description for a script that reports the comment at the caret.

description=_("Reports the text of the comment where the System caret is located.")

)

LeonarddeR · 2019-05-28T18:40:27Z

@@ -334,17 +345,20 @@ def script_reportCurrentComment(self,gesture):
 			UIAElement=UIAElementArray.getElement(index)


It really amazes me that this is not calling getCommentInfoFromPosition. Could you please change this in such a way that it uses that function and speaks a message based on what is in the dictionary? I think it is even possible to abstract this further and pas the result of CommentUIATextInfoQuickNavItem.label to ui.message.

If this sounds too complex, I"m happy to take this for you.

It works with getCommentInfoFromPosition but when I tried CommentUIATextInfoQuickNavItem.label I got its adress in a memory or "Property object is not callable" error.

I think it would be best to create a new _getPresentableCommentInfoFromPosition function that returns the label. Ten, you can make the label property return what that helper function returns for the particular textInfo, and same for the script.

Does that clear things up for you?

I hope I understood you well. If so, should this new function has its doc string?
I also added a message for no comments in script.

zstanecic · 2019-06-04T18:19:11Z

For me this build works as expected. Sorry for late reply.

jakubl7545 · 2019-06-07T06:37:18Z

@LeonarddeR Could you have another look on that?

LeonarddeR · 2019-07-30T09:25:12Z

@jakubl7545: Could you merge current master into this?

jakubl7545 · 2019-07-31T07:27:29Z

@LeonarddeR: done.

LeonarddeR · 2019-10-01T09:24:19Z

@jakubl7545 I'm sorry that it took me some time to come back to this. Could you plase merge master again and run scons lint base=origin/master to make sure this doesn't introduce linting errors?

jakubl7545 · 2019-10-18T13:18:54Z

@LeonarddeR It's done. Sorry for such a long delay.

LeonarddeR

Thanks @jakubl7545 , looks good

jakubl7545 · 2019-12-16T11:53:21Z

@michaelDCurran Would it be possible to merge this PR into 2019.3? It passed a review from @LeonarddeR so I think it's ready.

codeofdusk · 2020-08-26T23:21:29Z

Any updates on this PR? CC @michaelDCurran.

lukaszgo1 · 2020-11-15T21:56:40Z

This has been reported several times and it looks like this PR is just awaiting review. @michaelDCurran Do you intent to look at this one?

michaelDCurran · 2020-11-16T01:01:55Z

I'm quite worried that this implementation might have broken with newer builds of Office 365. I shall give it a test and report back.

cary-rowen · 2020-11-20T16:17:18Z

What is the latest news of this PR and is it possible to be merged in 2020.4?
Thanks.

lukaszgo1 · 2021-01-14T13:36:18Z

@michaelDCurran Ping. This has been asked about again in #11989

cary-rowen · 2021-01-15T03:04:31Z

@michaelDCurran Can you review this

michaelDCurran · 2021-01-15T03:13:43Z

This is not reporting comments for me in MS Word build 16.0.13707 nor has it been for many months. Can someone else confirm that this code still works in Microsoft Word for them? We need to also work out what MS Word version this broke in, and perhaps try and come up with a solution that works for both.

michaelDCurran · 2021-01-15T03:18:26Z

This pr does not work for me with Microsoft Word build 16.0.13707, nor has it worked with builds of MS Word that I have had for some months now. I am right now looking into how we can make this code more robust. At very least, as the annotation pattern is available, we can actually use that pattern to get the needed info directly now.

michaelDCurran · 2021-01-15T07:23:19Z

Here is an updated copy of getCommentInfoFromPosition which uses annotation properties directly, and does not need to walk as many objects. This code works on my 16.0.13707 build of MS Word:

def getCommentInfoFromPosition(position):
	"""
	Fetches information about the comment located at the given position in a word document.
	@param position: a TextInfo representing the span of the comment in the word document.
	@type L{TextInfo}
	@return: A dictionary containing keys of comment, author and date
	@rtype: dict
	"""
	val=position._rangeObj.getAttributeValue(UIAHandler.UIA_AnnotationObjectsAttributeId)
	if not val:
		return
	try:
		UIAElementArray=val.QueryInterface(UIAHandler.IUIAutomationElementArray)
	except COMError:
		return
	for index in range(UIAElementArray.length):
		UIAElement=UIAElementArray.getElement(index)
		UIAElement=UIAElement.buildUpdatedCache(UIAHandler.handler.baseCacheRequest)
		typeID = UIAElement.GetCurrentPropertyValue(UIAHandler.UIA_AnnotationAnnotationTypeIdPropertyId)
		if typeID == UIAHandler.AnnotationType_Comment:
			comment = UIAElement.GetCurrentPropertyValue(UIAHandler.UIA_NamePropertyId)
			author = UIAElement.GetCurrentPropertyValue(UIAHandler.UIA_AnnotationAuthorPropertyId)
			date = UIAElement.GetCurrentPropertyValue(UIAHandler.UIA_AnnotationDateTimePropertyId)
			return dict(comment=comment,author=author,date=date)

However, as MS Word now supports comment threads, much of the time, the comment reported will just be "comment thread started by Michael Curran" or similar. Users should really now start viewing comments in the comment thread pain.

jakubl7545 · 2021-01-15T22:00:40Z

Can someone else confirm that this code still works in Microsoft Word for them

@michaelDCurran Yes, it works for me in MSO (16.0.13530.20368). It reads comment itself, author and date exactly as it should.

cary-rowen · 2021-01-16T01:10:35Z

It works fine for me: Microsoft 365 MSO (16.0.13530.20132) 64 Bit
@michaelDCurran

AppVeyorBot · 2021-02-02T16:19:21Z

PR introduces Flake8 errors
Build (for testing PR)

See test results for failed build of commit 52eb987190

jakubl7545 · 2021-02-02T17:57:19Z

@michaelDCurran I've tried your code but it didn't work. Probably because AnnotationType_Comment is not available. Only AnnotationType_Unknown is returned. So I've combined both approaches to ensure that comments are read in different versions of Office. Could you check if it works for you now?

michaelDCurran · 2021-02-02T21:52:55Z

@jakubl7545 Yes, this is working for me. Thanks for your work on this.

michaelDCurran · 2021-02-02T22:15:01Z

@jakubl7545 I think there may be one or more review actions from @feerrenrut before he can also approve?

feerrenrut

All changes addressed

cary-rowen · 2021-08-04T05:53:40Z

In version Microsoft 365 MSO (16.0.14228.20200) 64-bit the pr reported an incorrect comment:
When I moved the cursor to the position containing the comment and pressed NVDA + Alt + C, NVDA reported the following:
"Comment: Resolved Comment Hint Button by Rowen Cary on 25 minutes ago"
According to the above, the content of the comment is: "Resolved Comment Hint" This is wrong and not the actual comment I wrote.
When I turned off the UIA mode in Word, everything returned to normal.
My language is Simplified Chinese, but I installed the English version of Office and can also reproduce this problem.

@feerrenrut
@michaelDCurran

lukaszgo1 · 2021-08-04T09:38:33Z

@cary-rowen Please create a new issue for this and preferably attach a sample document reproducing the problem.

jakubl7545 added 5 commits April 28, 2019 18:02

Change condition for filtering comments

d8a8cbf

Merge branch 'master' into i9285

25b62ee

Deal with unretrievable date

759a256

Split filtering condition to two separate lines

05b5620

Replace gesture dictionary with script decorator

2f8b6a3

LeonarddeR suggested changes May 28, 2019

View reviewed changes

jakubl7545 added 2 commits May 29, 2019 20:02

Adress review requests.

ee1aedc

Create helper function

0656372

MarcoZehe mentioned this pull request Jun 16, 2019

UIA in Word: Comments are not read when pressing NVDA+Alt+C #9755

Closed

Merge branch 'master' into i9285

f3e937e

Merge branch 'master' into i9285

117c830

This comment has been minimized.

Sign in to view

Address linting errors

53add22

This comment has been minimized.

Sign in to view

Address more linting errors

1e7f823

LeonarddeR previously approved these changes Oct 18, 2019

View reviewed changes

jakubl7545 added 2 commits December 16, 2019 10:28

Merge branch 'master' into i9285

7d45fa3

Modify copyright header

bb82408

Merge branch 'master' into i9285

5c8554a

feerrenrut added app/microsoft-office feature labels Apr 8, 2020

feerrenrut requested a review from michaelDCurran June 17, 2020 11:17

feerrenrut assigned michaelDCurran Jul 10, 2020

Merge branch 'master' into i9285

01f41d0

Ensure comments are read in different Office versions.

80c3b63

jakubl7545 dismissed LeonarddeR’s stale review via 80c3b63 February 2, 2021 16:07

Linting

ea0871a

michaelDCurran added 2 commits February 3, 2021 07:39

Merge branch 'master' into i9285

7d260e7

Update what's new

951e056

michaelDCurran approved these changes Feb 2, 2021

View reviewed changes

feerrenrut approved these changes Feb 3, 2021

View reviewed changes

michaelDCurran merged commit 8f31bdc into nvaccess:master Feb 3, 2021

nvaccessAuto added this to the 2021.1 milestone Feb 3, 2021

cary-rowen mentioned this pull request Aug 11, 2021

Enable UI Automation in Word Lead to confusion in comment reading #12713

Closed

-	@script(gesture ="kb:NVDA+alt+c")
+	@script(
+		gesture="kb:NVDA+alt+c",
+		# Translators: a description for a script that reports the comment at the caret.
+		description=_("Reports the text of the comment where the System caret is located.")
+	)

		@@ -334,17 +345,20 @@ def script_reportCurrentComment(self,gesture):
		UIAElement=UIAElementArray.getElement(index)

Uh oh!

Conversation

jakubl7545 commented May 28, 2019

Link to issue number:

Summary of the issue:

Description of how this pull request fixes the issue:

Testing performed:

Known issues with pull request:

Change log entry:

Uh oh!

zstanecic commented May 28, 2019 via email • edited by feerrenrut Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

lukaszgo1 commented May 28, 2019

Uh oh!

LeonarddeR left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

LeonarddeR May 28, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

zstanecic commented Jun 4, 2019 via email • edited by feerrenrut Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jakubl7545 commented Jun 7, 2019

Uh oh!

LeonarddeR commented Jul 30, 2019

Uh oh!

jakubl7545 commented Jul 31, 2019

Uh oh!

LeonarddeR commented Oct 1, 2019

Uh oh!

This comment has been minimized.

This comment has been minimized.

jakubl7545 commented Oct 18, 2019

Uh oh!

LeonarddeR left a comment

Choose a reason for hiding this comment

Uh oh!

jakubl7545 commented Dec 16, 2019

Uh oh!

codeofdusk commented Aug 26, 2020

Uh oh!

lukaszgo1 commented Nov 15, 2020

Uh oh!

michaelDCurran commented Nov 16, 2020 via email

Uh oh!

cary-rowen commented Nov 20, 2020

Uh oh!

lukaszgo1 commented Jan 14, 2021

Uh oh!

cary-rowen commented Jan 15, 2021

Uh oh!

michaelDCurran commented Jan 15, 2021

Uh oh!

michaelDCurran commented Jan 15, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

zstanecic commented May 28, 2019 via email •

edited by feerrenrut

Loading

LeonarddeR May 28, 2019 •

edited

Loading

zstanecic commented Jun 4, 2019 via email •

edited by feerrenrut

Loading

michaelDCurran commented Jan 15, 2021 •

edited

Loading

michaelDCurran commented Jan 15, 2021 •

edited

Loading