Skip to content

Handle menu start events processed after the menu item#12709

Merged
seanbudd merged 6 commits into
betafrom
fix-12624
Aug 7, 2021
Merged

Handle menu start events processed after the menu item#12709
seanbudd merged 6 commits into
betafrom
fix-12624

Conversation

@seanbudd

@seanbudd seanbudd commented Aug 4, 2021

Copy link
Copy Markdown
Member

Link to issue number:

Fixes #12624

Summary of the issue:

When opening a submenu in certain applications (like Thunderbird 78.12),
NVDA can process a menu start event after the first item in the menu is focused.
The menu start event causes a focus event on the menu, taking NVDA's focus from the menu item.
Additionally, the "menu" parent of the submenu item is not keyboard focusable, and is separate from
the menu item which triggered the submenu.
The object tree in this case (menu item > submenu (not keyboard focusable) > submenu item).
The focus event order after activating the menu item's sub menu is (submenu item, submenu).

Description of how this pull request fixes the issue:

Before cancelling speech, check that the focus is now on the parent menu.

Testing strategy:

Manual testing (note that this does not always occur and is hard to consistently reproduce):

  1. Enable debug logging
  2. With a Thunderbird 78.12 context menu (activate one on an email, for example)
  3. navigate to a menu item with a submenu.
  4. Activate it.
  5. Examine the logs for an event (submenu item, submenu) - eg the speech is announced in this order in the logs, as are the events if event debugging is enabled.
    On this PR: Confirm that this case has extensive logging when it occurs. When this event occurs confirm that the menu item is spoken, followed by the submenu. If you want to hear a beep on this event, change the logging to an error beep.
    On 2021.1: Confirm that only menu item is spoken, as the seubmenu item is cancelled.

Known issues with pull request:

This may be an overzealous check. Speech in similar contexts that should be cancelled may continue to be spoken.
As such, debug logs are added to make handling new cases easier until the root problem of event processing is addressed.

Change log entries:

Bug Fixes

- Fixed bug where the first menu item is not announced in a submenu in some contexts. (#12624)

Code Review Checklist:

  • Pull Request description is up to date.
  • Unit tests.
  • System (end to end) tests.
  • Manual testing.
  • User Documentation.
  • Change log entry.
  • Context sensitive help for GUI changes.
  • UX of all users considered:
    • Speech
    • Braille
    • Low Vision
    • Different web browsers

@seanbudd seanbudd requested a review from feerrenrut August 4, 2021 08:42
@seanbudd seanbudd requested a review from a team as a code owner August 4, 2021 08:42
@seanbudd seanbudd added this to the 2021.2 milestone Aug 4, 2021
@AppVeyorBot

Copy link
Copy Markdown

See test results for failed build of commit e8e80b203d

@lukaszgo1

Copy link
Copy Markdown
Contributor

I'm not sure this would be a complete fix for #12624 - its original STR mentioned context menu in Outlook which depending on the version (unfortunately @CyrilleB79 forgot to mention version of his Outlook) can be UIA not IAccessible. Were you able to reproduce the Outlook issue?

@seanbudd

seanbudd commented Aug 4, 2021

Copy link
Copy Markdown
Member Author

Were you able to reproduce the Outlook issue?

I could not reproduce this in my version of Outlook, I realised I mistakenly deleted a comment on the issue which mentioned that, so I've reposted it on the issue (see #12624 (comment)).

@CyrilleB79 - could you confirm this PR build works to fix the issue on your version of Outlook, as well list the Outlook version info?

feerrenrut
feerrenrut previously approved these changes Aug 5, 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.

Looks good, can you confirm this fixes the Thunderbird case?

This may be overzealous and lead to less unwanted speech being cancelled.

I'm struggling to parse this, is the concern it will lead to more unwanted speech?

@seanbudd

seanbudd commented Aug 6, 2021

Copy link
Copy Markdown
Member Author

Yep, the Thunderbird case it what I tested with and I can confirm this fixes it.

This is just noting that if any other scenario matches the check, speech won't be cancelled. I've rephrased the comment to

This may be an overzealous check. Speech in similar contexts that should be cancelled may continue to be spoken.

Because I updated the changes, this will need reapproval.

@seanbudd seanbudd requested a review from feerrenrut August 6, 2021 00:12
@seanbudd seanbudd merged commit afc9311 into beta Aug 7, 2021
@seanbudd seanbudd deleted the fix-12624 branch August 7, 2021 06:52
@seanbudd seanbudd linked an issue Aug 7, 2021 that may be closed by this pull request
@CyrilleB79

Copy link
Copy Markdown
Contributor

@CyrilleB79 - could you confirm this PR build works to fix the issue on your version of Outlook, as well list the Outlook version info?

Unfortunately NVDA 2021.2beta1 does not fix the Outlook issue. The version of Outlook is Outlook 2016 version 16.0.5182.1000. More details in #12624 (comment).

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.

First item not read when opening context submenu

5 participants