Skip to content

Fix context menu focus for Edge downloads#15300

Merged
seanbudd merged 6 commits into
nvaccess:masterfrom
LeonarddeR:edgeFix
Aug 21, 2023
Merged

Fix context menu focus for Edge downloads#15300
seanbudd merged 6 commits into
nvaccess:masterfrom
LeonarddeR:edgeFix

Conversation

@LeonarddeR

@LeonarddeR LeonarddeR commented Aug 15, 2023 •
edited
Loading

Copy link
Copy Markdown
Collaborator

Note, created against beta because this issue can be pretty annoying and it is a relatively small fix with a major positive impact for Edge users.

Link to issue number:

Fixes #14916

Summary of the issue:

When opening the context menu from the Edge downloads window, NVDA doesn't reflect this.

Description of user facing changes

Context menu is read again.

Description of development approach

  1. It looks like the context menu items aren't direct children of the foreground window, therefore we compare the root of the foreground and the root of the focus window instead.
  2. Fixed a typo in a UIAHandler debug string
  3. Ensured that UIAHandler.getNearestWindowHandle returns when a cached window handle couldn't be fetched from an UIAElement due to a COMError.

Testing strategy:

Tested that the context menu of a download item in Edge reads again.

Known issues with pull request:

The first item in the menu is read twice. I've tried to fix this without success. It looks like our duplicate focus event logic doesn't kick in properly. Note that Edge firing duplicate events is a bug on their end anyway.

Change log entries:

Bug fixes

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
  • Security precautions taken.

@LeonarddeR LeonarddeR requested a review from a team as a code owner August 15, 2023 20:43
@LeonarddeR LeonarddeR requested review from seanbudd and removed request for a team August 15, 2023 20:43
@LeonarddeR LeonarddeR mentioned this pull request Aug 15, 2023
Closed
@AppVeyorBot

AppVeyorBot commented Aug 15, 2023

Copy link
Copy Markdown

See test results for failed build of commit 5da41e4e78

@Carlos-EstebanM

Carlos-EstebanM commented Aug 15, 2023 •
edited
Loading

Copy link
Copy Markdown

Hello all.
I tested the compilation of this PR in a virtual machine with windows 11 and Microsoft Edge 115.0.1901.203 stable. NVDA cannot read the context menu, but there is not an error in the log, check with information and debug level. I also install and test with Microsoft Edge beta 116.0.1938.43, with the same result. I test with a clean and install copy on the virtual machine, and a friend test in his physical machine with the portable and temporally copy of this compilation, same versions of Windows and Microsoft Edge.
Regards.

@cary-rowen

cary-rowen commented Aug 16, 2023

Copy link
Copy Markdown
Contributor

Hi,

For me that fix worked.
MSEdge-115.0.1901.203
Windows 10 22H2 (AMD64) build 19045.3324

@LeonarddeR

LeonarddeR commented Aug 16, 2023

Copy link
Copy Markdown
Collaborator Author

@Carlos-EstebanM Thanks for reporting this. Here it works as expected. Could you please ensure the following?

  1. It is indeed the right build you're testing, namely this one
  2. You're using the right steps to reproduce, i.e.
    1. Open Edge
    2. Press CTRL+J to open the downloads window
    3. To ensure proper navigation, disable browse mode if it is enabled (NVDA+SPace)
    4. Focus a download (either completed or running)
    5. Press shift+f10 or the applications key.

If this doesn't work, please provide a log as follows:

  1. Set NVDA log level to debug
  2. In the advanced settings category, debug logging grouping, enable the UIA category
  3. Focus a download in Edge
  4. Press control+shift+nvda+f1 to start log collection
  5. Press either applications key or shift+f10 to open the context menu
  6. Press control+shift+nvda+f1 again to copy the relevant log snippet to the clip board
  7. Paste the snippet in this issue

@XLTechie

XLTechie commented Aug 16, 2023

Copy link
Copy Markdown
Collaborator

@LeonarddeR I also don't seem to be having success with this. That said, I'm not an Edge user, normally, so perhaps I'm doing it wrong.
Pressing Ctrl+j, Going into focus mode, tab to the file name, press context key. Nothing happens, can't shift+tab, NVDA is silent.
Edge 115.0.1901.203
Windows 10.0.22621.1 (11 Home 22H2)
Running a temporary copy of this PR

Log Attached

@LeonarddeR

LeonarddeR commented Aug 16, 2023

Copy link
Copy Markdown
Collaborator Author

It looks like in your case the window class is Chrome_WidgetWin_1, not Chrome_WidgetWin_2. I changed the code to support all incarnations of Chrome_WidgetWin_*

LeonarddeR
LeonarddeR commented Aug 16, 2023
View reviewed changes
Comment thread source/UIAHandler/__init__.py
@rperez030

rperez030 commented Aug 16, 2023

Copy link
Copy Markdown

It works for me with the latest commit. like @carlos, I also have an instance with the 'Chrome_WidgetWin_1' class. It also fixed the Folder menu in the add or edit favorite dialog as I expected. So far so good. Thanks @LeonarddeR for such a quick turnaround.

@cary-rowen

cary-rowen commented Aug 16, 2023

Copy link
Copy Markdown
Contributor

Hi,
I retested the latest commit and it still works.
Thanks

@seanbudd seanbudd changed the base branch from beta to master August 18, 2023 01:28
@seanbudd seanbudd added this to the 2023.3 milestone Aug 18, 2023
@seanbudd

seanbudd commented Aug 18, 2023

Copy link
Copy Markdown
Member

Apologies but this is much too late for a beta at this point. Unless another serious regression is found, we don't plan on releasing another beta.

@cary-rowen

cary-rowen commented Aug 18, 2023

Copy link
Copy Markdown
Contributor

But I think this bug seriously affects the experience of NVDA users. This is a bug specific to NVDA and Edge.
Although I'm not an Edge user, I'd like to see this fix in the latest stable release.

@wmhn1872265132

wmhn1872265132 commented Aug 18, 2023

Copy link
Copy Markdown
Contributor

Since Microsoft Edge is the pre-installed browser of Windows 10 and 11, this problem may affect many users, and the scenarios involved are also critical. I really hope to consider fixing it in time in this version

seanbudd
seanbudd reviewed Aug 18, 2023
View reviewed changes
Comment thread source/UIAHandler/__init__.py
Comment thread source/eventHandler.py
@seanbudd seanbudd marked this pull request as draft August 18, 2023 04:52
@XLTechie

XLTechie commented Aug 18, 2023 via email

Copy link
Copy Markdown
Collaborator

@LeonarddeR LeonarddeR marked this pull request as ready for review August 18, 2023 06:22
@seanbudd

seanbudd commented Aug 18, 2023 •
edited
Loading

Copy link
Copy Markdown
Member

It seems that a bad merge from master occurred - there are a lot of unrelated changes in the diff

@seanbudd

seanbudd commented Aug 18, 2023

Copy link
Copy Markdown
Member

ah - I see it was a merge from beta - can this be rebased on master instead?

@XLTechie XLTechie mentioned this pull request Aug 18, 2023
Closed
@LeonarddeR

LeonarddeR commented Aug 18, 2023

Copy link
Copy Markdown
Collaborator Author

ah - I see it was a merge from beta - can this be rebased on master instead?

I don't think there's consensus yet about whether this has to go to 2023.2, 2023.3 or a potential 2023.2.1 😉

@cary-rowen

cary-rowen commented Aug 18, 2023

Copy link
Copy Markdown
Contributor

Hi @seanbudd

  1. A user raised this topic on Groups.io early on and asked if there is an issue about this bug being opened on github.
  2. Even on github, there are multiple Issues related to this bug that have been created successively.
    Such as: Microsoft edge: unable to redirect focus to context menu in downloads window #14916, Some context menus in edge have become inaccessible with NVDA #15293, edge: information not announced after activating 'More actions' control. #15153, NVDA doesn't announce the context menus in the downloads window of microsoft edge #15055, Cannot access context menu when browsing downloads in Edge with control+j #14922

Based on the above, I think community users are suffering from this bug, can you reconsider giving this fix to beta branch?

Thanks

@Nikita34196

Nikita34196 commented Aug 18, 2023

Copy link
Copy Markdown

Hello. I personally use nvda and edge. I am one of those suffering from this problem. I would like this to be released as soon as possible. I have many friends who use microsoft edge and this problem is quite annoying.

@rperez030

rperez030 commented Aug 18, 2023

Copy link
Copy Markdown

Apologies but this is much too late for a beta at this point. Unless another serious regression is found, we don't plan on releasing another beta.

In my opinion, a bug like this even deserves a hotfix. It affects multiple areas in the default browser. It can fully block at least 1 use case, in this case allowing the download of unsigned executables that are blocked by Windows defender. Users shouldn't wait an entire quarter or be forced to use development builds or install an add-on to get around an issue that blocks functionality in the default browser.

@seanbudd seanbudd merged commit 9e8e419 into nvaccess:master Aug 21, 2023
@cary-rowen

cary-rowen commented Aug 21, 2023

Copy link
Copy Markdown
Contributor

@seanbudd
Did you see the comment above? Can NV Access reconsider fixing this bug in 2023.2?

@LeonarddeR LeonarddeR mentioned this pull request Sep 16, 2023
Merged
7 tasks
seanbudd pushed a commit that referenced this pull request Sep 20, 2023
@LeonarddeR
Fix more cases of grand child window focus failures (#15455)
262b27c 
Closes #15432

Summary of the issue:
Some events for grand child windows in applications seem to be ignored.

Description of user facing changes
Focus changes are reported in these cases.

Description of development approach
Generalize the workaround that was expanded for Edge in #15300
@XLTechie XLTechie mentioned this pull request Sep 11, 2024
Closed
@LeonarddeR LeonarddeR deleted the edgeFix branch August 23, 2025 06:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@seanbudd seanbudd seanbudd approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

2023.3

Development

Successfully merging this pull request may close these issues.

Microsoft edge: unable to redirect focus to context menu in downloads window

9 participants

@LeonarddeR @AppVeyorBot @Carlos-EstebanM @cary-rowen @XLTechie @rperez030 @seanbudd @wmhn1872265132 @Nikita34196