Skip to content

Update braille when control+v, control+x, control+y, control+z, backspace or control+backspace is pressed and UIA not used in ms word #15491

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 30 commits into from
Oct 18, 2023

Conversation

burmancomp
Copy link
Contributor

@burmancomp burmancomp commented Sep 21, 2023

Link to issue number:

fixes #3276

Summary of the issue:

After paste, cut, redo, undo, backspace or control+backspace braille line was not necessarily updated when using object model.

Description of user facing changes

Braille line and review position should always be updated when using object model ("Use UI Automation to access Microsoft Word document controls" is not set to always in NVDA's advanced settings category), and paste (control+v), cut (control+x), redo (control+y) or undo (control+z / alt+backspace) is pressed. Braille should be also updated when using object model and backspace or control+backspace is pressed and held down.

Braille and review position are also updated when using UIA, and braille is tethered to review and review follows caret.

Description of development approach

To get control+v, control+x, control+y, control+z and alt+backspace to work, added helper script script_updateBrailleAndReviewPosition to NVDAObjects.IAccessible\winword.WordDocument, and assigned corresponding keyboard shortcuts to it. Minor modification of event_caret so that braille is always updated when caret event is executed from script_updateBrailleAndReviewPosition. To update braille always when backspace or control+backspace are pressed and held down, overridden _backspaceScriptHelper function.

In addition, to get braille updated when tethered to review and when review follows caret, modified event_textChange in UIA\wordDocument.py.

Testing strategy:

Tested with word 2019 (version 2308 Build 16.0.16731.20182) 32-bit using Albatross 80. More testing with various configurations would be needed.

Known issues with pull request:

none at this moment

Code Review Checklist:

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

@burmancomp
Copy link
Contributor Author

cc: @LeonarddeR

@@ -47,12 +47,6 @@ def _get_ignoreEditorRevisions(self):
ignoreFormatting=False

def event_caret(self):
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think we need to research why this code was ever added. It should be possible to do this with a git blame.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@michaelDCurran if super(WordDocument,self).event_caret() is not executed there seems to be problem that braille is not always updated when backspace is pressed (problem should occur at least if backspace is pressed and hold down).

Copy link
Contributor

Choose a reason for hiding this comment

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

I tent to agree with @LeonarddeR that checking with git blame in which commit this was introduced is safer. Would you be able to do this?


def event_UIA_notification(self, activityId=None, **kwargs):
# #10851: in recent Word 365 releases, UIA notification will cause NVDA to announce edit functions
# such as "delete back word" when Control+Backspace is pressed.
if activityId == "AccSN2": # Delete activity ID
return
super(WordDocument, self).event_UIA_notification(**kwargs)
# Try to ensure that braille is updated when UIA is not used and
Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't think it is a good idea to do this here. There must be another event we're missing, and if not, it's simply a bug in Word that needs to be reported and fixed on their end.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It seems that sometimes event_caret (IAccessible\winword.py) is fired when control+z is pressed and sometimes not. UIA_notification event seems to be fired always.

I think I have tried to add textChange (and textInsert/textRemove) events to IAccessible\winword.py but I suppose they are not fired.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Do you intend this to work for Non UIA cases with this code? If so I think it's a false positive, because this code is not active when UIA is disabled.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

When "Use UI Automation to access Microsoft Word document controls Only when necessary" event_UIA_notification is fired when pressing control+x and control+z.

Can reason be that object model is not available for some reason and thus UIA is used?

Copy link
Collaborator

Choose a reason for hiding this comment

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

If a notification event is fired for undo but neither a caret nor a text change event is fired, this is something Microsoft needs to fix.

@CyrilleB79
Copy link
Contributor

Have you double checked other editing commands: del, shift+del, shift+backspace, redo (control+Y), etc.?

@CyrilleB79
Copy link
Contributor

@burmancomp, keep in mind that "Use UI Automation to access Microsoft Word document controls" only applies to situations when NVDA queries informations such as curent line, table, current text formatting, etc.

I does not apply to event triggered by an action and thus to UIA notifications. So does not apply to notifications received when performing control+Z or control+U.

Also to check if you are using UIA or object model to access Word document, you can press NVDA+F1 and look at dev info at the end of the log. If the dev info ends with UIA peroperties, you are accessing the document via UIA; else, if it ends with IAccessible properties, you are accessing it through object model.

At last, beware to #13704 when testing with Word: be sure to change the focus at least once before doing tests.

@LeonarddeR
Copy link
Collaborator

I does not apply to event triggered by an action and thus to UIA notifications. So does not apply to notifications received when performing control+Z or control+U.

I don't think that's true. Word sends UIA notifications to its UIA control, so when that control is used, NVDA will receive those. If the object model is active and uIA is disabled for Word, the notifications will still be send but NVDA silently ignores them anyway.

@CyrilleB79
Copy link
Contributor

I does not apply to event triggered by an action and thus to UIA notifications. So does not apply to notifications received when performing control+Z or control+U.

I don't think that's true. Word sends UIA notifications to its UIA control, so when that control is used, NVDA will receive those. If the object model is active and uIA is disabled for Word, the notifications will still be send but NVDA silently ignores them anyway.

@LeonarddeR, I have just tested with Word 2016, version '16.0.16731.20234', with "Use UIA" option to "Only when necessary", and I have double checked that UIA is actually not used with looking at dev info in the log viewer. I can confirm that even if NVDA uses object model to access Word document, the following actions trigger UIA notifications:

  • copy (control+C), cut (ctrl+X), paste (ctrl+V)
  • cancel (ctrl+Z), redo (ctrl+Y)
  • formatting bold (ctrl+G in French), italic (ctrl+I) and underline (ctrl+U): in this last case, both UIA notifications and NVDA's ui.message in formatting scripts are spoken.

@LeonarddeR
Copy link
Collaborator

@LeonarddeR, I have just tested with Word 2016, version '16.0.16731.20234', with "Use UIA" option to "Only when necessary", and I have double checked that UIA is actually not used with looking at dev info in the log viewer. I can confirm that even if NVDA uses object model to access Word document, the following actions trigger UIA notifications:

Wow, that's pretty spooky. In any case, that means we should not touch the notification code for now.

@burmancomp
Copy link
Contributor Author

If event_UIA_notification code is not touched how braille is updated? I am suspicious that Microsoft would fix this. It is issue of older object model.

@seanbudd
Copy link
Member

Please update the PR title to be more descriptive

@burmancomp burmancomp changed the title fix 3276 Update braille when control+z, control+x or backspace is pressed and UIA not used Sep 22, 2023
@burmancomp burmancomp changed the title Update braille when control+z, control+x or backspace is pressed and UIA not used Update braille when control+z, control+x or backspace is pressed and UIA not used in ms word Sep 22, 2023
@seanbudd seanbudd added this to the 2024.1 milestone Sep 26, 2023
@burmancomp burmancomp changed the title Update braille when control+z, control+x or backspace is pressed and UIA not used in ms word Update braille when control+v, control+x, control+z or backspace is pressed and UIA not used in ms word Sep 26, 2023
@burmancomp
Copy link
Contributor Author

@LeonarddeR what do you think about code now?

@LeonarddeR
Copy link
Collaborator

This is not going to work for older version of Word that don't provide UIA notifications.

@burmancomp
Copy link
Contributor Author

How old office versions NVDA should support (is there official policy)? Support of office 2010 ended on october 2020, and support of office 2013 ended in april 2023.

@CyrilleB79 told in his comment about UIA notifications.

This pull request could be limited to try to solve issue from 2016.

@AppVeyorBot
Copy link

See test results for failed build of commit cc1375fb40

@AppVeyorBot
Copy link

See test results for failed build of commit 51a93734a0

@AppVeyorBot
Copy link

See test results for failed build of commit c9815650e8

@burmancomp burmancomp changed the title Update braille when control+v, control+x, control+z or backspace is pressed and UIA not used in ms word Update braille when control+v, control+x, control+y, control+z, backspace or control+backspace is pressed and UIA not used in ms word Oct 7, 2023
@burmancomp
Copy link
Contributor Author

As to letters y or z sometimes typed when control+y or control+z is pressed and held down, I suppose it is not issue of this pull request.

I have noticed that when backspace or control+backspace are pressed, held down and then released speech may be inaccurate (not reporting most recent deletion). This may happen also when using current main branch so I suppose this behavior is not merely issue of this pull request.

@burmancomp burmancomp marked this pull request as ready for review October 7, 2023 06:40
@seanbudd
Copy link
Member

seanbudd commented Oct 9, 2023

Please put the word "fixes" or "closes" before the issue number in the PR description so GitHub automatically links the issue

@seanbudd seanbudd marked this pull request as draft October 12, 2023 02:27
@seanbudd seanbudd added the conceptApproved Similar 'triaged' for issues, PR accepted in theory, implementation needs review. label Oct 16, 2023
@burmancomp burmancomp marked this pull request as ready for review October 18, 2023 09:20
Copy link
Contributor

@lukaszgo1 lukaszgo1 left a comment

Choose a reason for hiding this comment

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

LGTM!

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 @burmancomp

@seanbudd seanbudd merged commit 7e9f9c7 into nvaccess:master Oct 18, 2023
@burmancomp burmancomp deleted the fix_3276 branch October 19, 2023 06:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
conceptApproved Similar 'triaged' for issues, PR accepted in theory, implementation needs review.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Text on the braille display doesn't update after using Undo in Word
6 participants