Skip to content

Support the Modern Comments side track pane in MS Word when not accessing MS Word documents via UIA #12988

Merged
michaelDCurran merged 5 commits into
betafrom
i12982
Oct 27, 2021
Merged

Support the Modern Comments side track pane in MS Word when not accessing MS Word documents via UIA #12988
michaelDCurran merged 5 commits into
betafrom
i12982

Conversation

@michaelDCurran

@michaelDCurran michaelDCurran commented Oct 26, 2021

Copy link
Copy Markdown
Member

Link to issue number:

Fixes #12982

Summary of the issue:

In build 13901, MS Word introduced "Modern comments" which allows comment threads (I.e. an initial comment plus replies) and the ability to resolve or reopen threads. The new UI to do this is exposed as a new comments side pane which shows the comments and replies in a treeview. This UI is only exposed via UI Automation.
This UI is supposed to completely replace the older Comments story in a document.
As this new UI is imbedded within the document, it shares the same window handle. If NvDA is using the object model to access the document (I.e. the UIA provider of the document's window is ignored) then NVDA never sees the modern comments UI at all. It seems that MS Word does not ever proxy this to MSAA either.
Also, if MS Word detects that something is trying to access the comments via the object model in the old way (E.g. calls comment.range) then the Modern comments UI is disabled (hidden) and MS Word ends up in a state where querying the range at the selection of the object model is no longer in the comments story, yet the UI is not shown either. This MS Word bug is known to Microsoft however they have no plans or willingness to fix it. Their only recommended solution is to stop using the object model and just use UIA.

Description of how this pull request fixes the issue:

in UIAHandler.handler.isUIAElement: return true if the element's windowHandle is _WwG but an ancestor of the element has a UIA className of NetUIHWNDElement.
I.e. this UIA element is a control of a NetUI container embedded in an MS Word document. E.g. a control in the Modern Comments side track pane.
Also, if there is a UIA focus event for the Word document root, even though we are not classing that window as native UIA, if the previously focused object was a control in a NetUI container embedded in the same Word document, then generate an MSAA focus event for the document, as MS Word will not do this itself.

Note that allowing NVDA to track the focus with UIA for these embedded controls is enough to work around the issue, as this then means that focus never hits the Word document while the Comments side track pane should be focused, thus avoids making object model calls MS Word does not like or expect.

Testing strategy:

This can only be tested in conjunction with PR #12989 as UIA in MS word should not be used by default.

  1. Ensure that the NVDA advanced setting: Use UIA in MS Word document controls when available is turned off.
  2. Open a new MS Word document.
  3. Ensure the build of MS Word is 13901 or later.
  4. Type some text. E.g. "this is a test".
  5. Select a word, E.g. "This".
  6. In the context menu, choose "New Comment".
  7. Ensure that focus lands in the Modern Comments side track pane ready for you to type the comment.
  8. Type a comment, and tab to the Post comment button and press enter.
  9. press alt+f12 to move back to the document and ensure that the arrow keys cause the caret to move and NVDA speaks the characters.
  10. Again, select the same word you created the comment for.
  11. Press alt+f12 to move to the Modern comment side track pane for this comment.

Known issues with pull request:

None known.

Change log entries:

New features
Changes
Bug fixes

  • The new Modern Comments side track pane is now supported in MS Word when not accessing the document via UIA. Press alt+f12 to move between the side track pane and the document.
    For Developers

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

@michaelDCurran michaelDCurran requested a review from a team as a code owner October 26, 2021 02:52
@michaelDCurran michaelDCurran requested review from seanbudd and removed request for a team October 26, 2021 02:52
@michaelDCurran michaelDCurran added this to the 2021.3 milestone Oct 26, 2021
@codeofdusk

codeofdusk commented Oct 26, 2021

Copy link
Copy Markdown
Contributor
  1. Ensure that the NVDA advanced setting: Use UIA in MS Word document controls when available is turned off.

In build 13901+, UIA cannot be turned off. Maybe make the setting ternary as with UIA in consoles and make a config spec upgrade?

Edit: never mind, looks like the intention in #12982 is to rollback UIA by default.

@michaelDCurran

michaelDCurran commented Oct 26, 2021 via email

Copy link
Copy Markdown
Member Author

seanbudd
seanbudd previously approved these changes Oct 26, 2021
Comment thread source/_UIAHandler.py Outdated
Comment thread source/_UIAHandler.py Outdated
Comment thread source/_UIAHandler.py
and self.getNearestWindowHandle(element) == oldFocus.windowHandle
and not self.isUIAWindow(oldFocus.windowHandle)
):
IAccessibleHandler.internalWinEventHandler.winEventLimiter.addEvent(

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Can't we set focus by calling accSelect here?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

It already does have focus according to Microsoft Word, it is just that it doesn't fire the focus winEvent.

Comment thread source/_UIAHandler.py Outdated
Co-authored-by: Leonard de Ruijter <leonardder@users.noreply.github.com>
@michaelDCurran michaelDCurran changed the title Support the Modern Comments side track pain in MS Word when not accessing MS Word documents via UIA Support the Modern Comments side track pane in MS Word when not accessing MS Word documents via UIA Oct 26, 2021
Comment thread source/_UIAHandler.py Outdated
if getattr(element, '_isNetUIEmbeddedInWordDoc', False):
return True
windowHandle = self.getNearestWindowHandle(element)
if winUser.getClassName(windowHandle) != '_WwG':

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

_WwG, _WwN, _WwO are used many times throughout the code base, could you put these in a constant?
The advantage would be a name and docs, hopefully giving insight to someone viewing this later and likely having a hard time reversing this class name to work out what control/window did you specifically care about when writing this code. At least a comment to give an indication for how this magic value is found.

Comment thread source/_UIAHandler.py
# But, if focus jumps from one of these controls back to the document (E.g. the user presses escape),
# we receive no MSAA focus event, only a UIA focus event.
# As we are not treating the Word doc as UIA, we need to manually fire an MSAA focus event on the document.
self._emitMSAAFocusForWordDocIfNecessary(sender)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I'm uncomfortable with word specific code being in this file. This seems like it should be a very general file?
Not something to deal with now (unless it is easy), but something to think about longer term.

Comment thread source/_UIAHandler.py Outdated
Comment on lines +871 to +872
cacheRequest.AddProperty(UIA.UIA_ClassNamePropertyId)
cacheRequest.addProperty(UIA.UIA_NativeWindowHandlePropertyId)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Why is one AddProperty with a cap 'A' and the other lower 'a'?

feerrenrut
feerrenrut previously approved these changes Oct 27, 2021

@feerrenrut feerrenrut left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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.

5 participants