-
-
Notifications
You must be signed in to change notification settings - Fork 823
revert changes to the destruction of systray from the mainframe #12243
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
Closed
Changes from all commits
Commits
Show all changes
16 commits
Select commit
Hold shift + click to select a range
b5547cb
revert changes to the destruction of systray from the mainframe
seanbudd 03c6d3d
add explanation comments
seanbudd 2888f55
Merge branch 'master' into fix-12238
seanbudd 9e9c6f1
add comments at untracked object creation
seanbudd 9f3240d
use safer event bind where possible
seanbudd a84dece
remove evt skip
seanbudd 7290f17
stop and resume event propagation
seanbudd a60cf4d
add comment to safeappexit
seanbudd 362d05e
use callafter to prevent crash
seanbudd b063e57
Merge branch 'master' into fix-12238
seanbudd 6e2b3c7
fix typo and comment for evt skip
seanbudd 593d89f
add better comments
seanbudd b5caece
Merge branch 'master' into fix-12238
seanbudd 4537051
fix formatter issues
seanbudd 4c5de0a
Merge branch 'master' into fix-12238
seanbudd aac13ad
Merge branch 'master' into fix-12238
seanbudd File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -53,7 +53,12 @@ def __init__(self): | |
| style = wx.DEFAULT_FRAME_STYLE ^ wx.MAXIMIZE_BOX ^ wx.MINIMIZE_BOX | wx.FRAME_NO_TASKBAR | ||
| super(MainFrame, self).__init__(None, wx.ID_ANY, versionInfo.name, size=(1,1), style=style) | ||
| self.Bind(wx.EVT_CLOSE, self.onExitCommand) | ||
|
|
||
| self.sysTrayIcon = SysTrayIcon(self) | ||
| # wx destroys child Windows automatically but `wx.adv.TaskBarIcon` is not a window | ||
| # so it must be set to be destroyed before destroying our main frame window (#12243) | ||
| self.Bind(wx.EVT_WINDOW_DESTROY, self._onDestroy, source=self) | ||
|
|
||
| #: The focus before the last popup or C{None} if unknown. | ||
| #: This is only valid before L{prePopup} is called, | ||
| #: so it should be used as early as possible in any popup that needs it. | ||
|
|
@@ -75,6 +80,13 @@ def __init__(self): | |
| self.Show() | ||
| self.Hide() | ||
|
|
||
| def _onDestroy(self, evt: wx.EVT_WINDOW_DESTROY): | ||
| evt.Skip(skip=False) # blocks other handlers as wxCommandEvents must only be processed by one handler | ||
| # wx destroys child Windows automatically but `wx.adv.TaskBarIcon` is not a window | ||
| # so it must be set to be destroyed when destroying our main frame window (#12243) | ||
| log.debug(f"destroying systray icon") | ||
| wx.CallAfter(self.sysTrayIcon.Destroy) # queue the destruction event safely | ||
|
|
||
| def prePopup(self): | ||
| """Prepare for a popup. | ||
| This should be called before any dialog or menu which should pop up for the user. | ||
|
|
@@ -358,30 +370,35 @@ def onConfigProfilesCommand(self, evt): | |
|
|
||
| def safeAppExit(): | ||
| """ | ||
| Ensures the app is exited by all the top windows being destroyed | ||
| Ensures the app is exited by all the top windows being destroyed. | ||
| wx objects that don't inherit from wx.Window (eg sysTrayIcon, Menu) need to be manually destroyed. | ||
| """ | ||
|
|
||
| for window in wx.GetTopLevelWindows(): | ||
| if isinstance(window, wx.Dialog) and window.IsModal(): | ||
| log.info(f"ending modal {window} during exit process") | ||
| log.debug(f"ending modal {window} during exit process") | ||
| wx.CallAfter(window.EndModal, wx.ID_CLOSE_ALL) | ||
| if isinstance(window, MainFrame): | ||
| log.info(f"destroying main frame during exit process") | ||
| log.debug(f"destroying main frame during exit process") | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same here w.r.t f-string being unnecessary. |
||
| # the MainFrame has EVT_CLOSE bound to the ExitDialog | ||
| # which calls this function on exit, so destroy this window | ||
| wx.CallAfter(window.Destroy) | ||
| else: | ||
| log.info(f"closing window {window} during exit process") | ||
| log.debug(f"closing window {window} during exit process") | ||
| wx.CallAfter(window.Close) | ||
|
|
||
|
|
||
| class SysTrayIcon(wx.adv.TaskBarIcon): | ||
|
|
||
| def __init__(self, frame): | ||
| super(SysTrayIcon, self).__init__() | ||
| icon=wx.Icon(ICON_PATH,wx.BITMAP_TYPE_ICO) | ||
| self.SetIcon(icon, versionInfo.name) | ||
|
|
||
| self.menu=wx.Menu() | ||
| # wx.Windows destroy child Windows automatically but wx.Menu and TaskBarIcon don't inherit from | ||
| # wx.Window. The menu must be manually destroyed when destroying our system tray icon (#12243) | ||
| self.menu = wx.Menu() | ||
|
|
||
| menu_preferences=self.preferencesMenu=wx.Menu() | ||
| item = menu_preferences.Append(wx.ID_ANY, | ||
| # Translators: The label for the menu item to open NVDA Settings dialog. | ||
|
|
@@ -542,6 +559,13 @@ def __init__(self, frame): | |
| self.Bind(wx.adv.EVT_TASKBAR_LEFT_DOWN, self.onActivate) | ||
| self.Bind(wx.adv.EVT_TASKBAR_RIGHT_DOWN, self.onActivate) | ||
|
|
||
| def Destroy(self): | ||
| # wx.Windows destroy child Windows automatically but wx.Menu and TaskBarIcon don't inherit from | ||
| # wx.Window. The menu must be manually destroyed when destroying our system tray icon (#12243) | ||
| log.debug(f"destroying systray menu during exit process") | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. And one more |
||
| wx.CallAfter(self.menu.Destroy) | ||
| return super().Destroy() | ||
|
|
||
| def onActivate(self, evt): | ||
| mainFrame.prePopup() | ||
| import appModules.nvda | ||
|
|
||
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why you're using f-string here?