Skip to content
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

Py3.8: allow NVDA scripts to again run when in a wx popup menu or message box. #12072

Merged
merged 1 commit into from
Feb 17, 2021

Conversation

michaelDCurran
Copy link
Member

@michaelDCurran michaelDCurran commented Feb 14, 2021

Link to issue number:

Fixes #12058

Summary of the issue:

If NVDA is run with wxPython 4.1.1, NVDA-specific key commands (scripts) fail to execute while in a WX popup menu (such as the NVDA menu) or wx message box (such as the NVDA About dialog). Yet, key commands that are not trapped by NVDA (E.g. pressing downArrow in a menu) still work okay.
In recent versions of wxWidgets, if in a menu or message box, calling wxWidget's CallAfter function does not execute the given callback until the menu or message box is dismissed (I.e. the wx main loop is again running). There are some exceptions to this, such as if a key or other input is sent to the application. But of course in the case of trying to execute an NVDA-specific command, NVDA blocks the key from getting to the application and therefore the wx event loop never wakes up.
In wxWidgets, it looks like CallAfter calls QueueEvent, which calls WakeupIdle.
Further investigation shows that queuing any kind of wx event (command, idle etc) does not wake up the wx event loop while in popup menus or message boxes.

Description of how this pull request fixes the issue:

Monkeypatch wx.CallAfter to post a WM_NULL message to our top-level window after calling the original CallAfter, which causes wx's event loop to wake up enough to execute the callback.

Testing strategy:

Note: this pr (nor the py3.8 branch at time of this comment) does not yet specifically use wxPython 4.1.1, thus it is necessary for testing that you first:

  • cd to include/wx
  • Delete everything in this directory
  • Fetch wxPython 4.1.1 into this directory with the command: py -m pip install -t . wxPython==4.1.1
    This pr has so far been tested manually by:
  • Starting NVDA
  • Executing the ReportTime NVDA script by pressing NVDA+f12 and noting that NVDA speaks the time.
  • Opening the NVDA menu with NVDA+n
  • Again executing reportTime script with NVDA+f12 and noting that NVDA speaks the time. previous to this pr, if using wxPython 4.1.1, the command would fail to execute at all.
  • Open NVDA menu -> Help -> About...
  • Again execute reportTime script with NVDA+f12 and note that NVDA speaks the time.
    Unit tests do not apply to this pr as the issue is within wxPython.
    System testing is certainly possible here. But only once wxPython 4.1.1 is in NVDA.

Known issues with pull request:

None.

Change log entry:

None needed.

Code Review Checklist:

This checklist is a reminder of things commonly forgotten in a new PR.
Authors, please do a self-review and confirm you have considered the following items.
Mark items you have considered by checking them.
You can do this when editing the Pull request description with an x: [ ] becomes [x].
You can also check the checkboxes after the PR is created.

  • Pull Request description is up to date.
  • Unit tests.
  • System (end to end) tests.
  • Manual tests.
  • User Documentation.
  • Change log entry.
  • Context sensitive help for GUI changes.

Sorry, something went wrong.

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
…ger executes callbacks while NVDA's main thread is within apopup menu or message box.

Monkeypatch wx.CallAfter to post a WM_NULL message to our top-level window after calling the original CallAfter, which causes wx's event loop to wake up enough to execute the callback.
Copy link
Collaborator

@josephsl josephsl left a comment

Choose a reason for hiding this comment

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

Thanks- tested manually myself and works, too.

@LeonarddeR
Copy link
Collaborator

Is this change in WX actually a bug? How did you find out this workaround?

@michaelDCurran
Copy link
Member Author

michaelDCurran commented Feb 15, 2021 via email

Copy link
Contributor

@feerrenrut feerrenrut left a comment

Choose a reason for hiding this comment

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

Looks fine to me.

I've been through the checklist, they can all be ticked.

Could you please raise an issue on the wxPython issue tracker? This will help us to remove the workaround if / when the issue is fixed.

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.

None yet

5 participants