Skip to content

Commit

Permalink
Audit qt dialogs connections (#536)
Browse files Browse the repository at this point in the history
* Audit qt dialog cleanup

* Fix AboutDialog super call

* Use lists to keep track and remove all connections

* Remove if control is None checks
  • Loading branch information
ievacerny authored Jun 19, 2020
1 parent d5c071e commit 5d16083
Show file tree
Hide file tree
Showing 4 changed files with 81 additions and 4 deletions.
8 changes: 8 additions & 0 deletions pyface/tests/test_progress_dialog.py
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,14 @@ def test_can_cancel(self):
with self.event_loop():
self.dialog.destroy()

def test_can_ok(self):
# test that creation works with can_ok
self.dialog.can_ok = True
with self.event_loop():
self.dialog._create()
with self.event_loop():
self.dialog.destroy()

def test_show_time(self):
# test that creation works with show_time
self.dialog.show_time = True
Expand Down
21 changes: 20 additions & 1 deletion pyface/ui/qt4/about_dialog.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
from pyface.qt import QtCore, QtGui


from traits.api import Instance, List, provides, Str
from traits.api import Any, Callable, Instance, List, provides, Str, Tuple


from pyface.i_about_dialog import IAboutDialog, MAboutDialog
Expand Down Expand Up @@ -71,6 +71,24 @@ class AboutDialog(MAboutDialog, Dialog):

image = Instance(ImageResource, ImageResource("about"))

# Private interface ---------------------------------------------------#

#: A list of connected Qt signals to be removed before destruction.
#: First item in the tuple is the Qt signal. The second item is the event
#: handler.
_connections_to_remove = List(Tuple(Any, Callable))

# -------------------------------------------------------------------------
# 'IWidget' interface.
# -------------------------------------------------------------------------

def destroy(self):
while self._connections_to_remove:
signal, handler = self._connections_to_remove.pop()
signal.disconnect(handler)

super(AboutDialog, self).destroy()

# ------------------------------------------------------------------------
# Protected 'IDialog' interface.
# ------------------------------------------------------------------------
Expand Down Expand Up @@ -99,6 +117,7 @@ def _create_contents(self, parent):
buttons.addButton(QtGui.QDialogButtonBox.Ok)

buttons.accepted.connect(parent.accept)
self._connections_to_remove.append((buttons.accepted, parent.accept))

lay = QtGui.QVBoxLayout()
lay.addWidget(label)
Expand Down
28 changes: 26 additions & 2 deletions pyface/ui/qt4/dialog.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,9 @@
from pyface.qt import QtCore, QtGui


from traits.api import Bool, Enum, Int, provides, Str

from traits.api import (
Any, Bool, Callable, Enum, Int, List, provides, Str, Tuple
)

from pyface.i_dialog import IDialog, MDialog
from pyface.constant import OK, CANCEL, YES, NO
Expand Down Expand Up @@ -60,6 +61,13 @@ class Dialog(MDialog, Window):

title = Str("Dialog")

# Private interface ---------------------------------------------------#

#: A list of connected Qt signals to be removed before destruction.
#: First item in the tuple is the Qt signal. The second item is the event
#: handler.
_connections_to_remove = List(Tuple(Any, Callable))

# ------------------------------------------------------------------------
# Protected 'IDialog' interface.
# ------------------------------------------------------------------------
Expand All @@ -77,6 +85,7 @@ def _create_buttons(self, parent):

btn.setDefault(True)
btn.clicked.connect(self.control.accept)
self._connections_to_remove.append((btn.clicked, self.control.accept))

# 'Cancel' button.
if self.cancel_label:
Expand All @@ -87,6 +96,7 @@ def _create_buttons(self, parent):
btn = buttons.addButton(QtGui.QDialogButtonBox.Cancel)

btn.clicked.connect(self.control.reject)
self._connections_to_remove.append((btn.clicked, self.control.reject))

# 'Help' button.
# FIXME v3: In the original code the only possible hook into the help
Expand Down Expand Up @@ -138,6 +148,17 @@ def _show_modal(self):
retval = dialog.exec_()
return _RESULT_MAP[retval]

# -------------------------------------------------------------------------
# 'IWidget' interface.
# -------------------------------------------------------------------------

def destroy(self):
while self._connections_to_remove:
signal, handler = self._connections_to_remove.pop()
signal.disconnect(handler)

super(Dialog, self).destroy()

# ------------------------------------------------------------------------
# Protected 'IWidget' interface.
# ------------------------------------------------------------------------
Expand All @@ -149,6 +170,9 @@ def _create_control(self, parent):
# MDialog's open method. For 'nonmodal', we do it here.
if self.style == "nonmodal":
dlg.finished.connect(self._finished_fired)
self._connections_to_remove.append(
(dlg.finished, self._finished_fired)
)

if self.size != (-1, -1):
dlg.resize(*self.size)
Expand Down
28 changes: 27 additions & 1 deletion pyface/ui/qt4/progress_dialog.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,9 @@

from pyface.qt import QtGui, QtCore

from traits.api import Bool, Instance, Int, Str, provides
from traits.api import (
Any, Bool, Callable, Instance, Int, List, Str, provides, Tuple
)

from pyface.i_progress_dialog import IProgressDialog, MProgressDialog
from .window import Window
Expand Down Expand Up @@ -81,6 +83,13 @@ class ProgressDialog(MProgressDialog, Window):
#: The widget showing the estimated time remaining
_remaining_control = Instance(QtGui.QLabel)

# Private interface ---------------------------------------------------#

#: A list of connected Qt signals to be removed before destruction.
#: First item in the tuple is the Qt signal. The second item is the event
#: handler.
_connections_to_remove = List(Tuple(Any, Callable))

# -------------------------------------------------------------------------
# IWindow Interface
# -------------------------------------------------------------------------
Expand All @@ -97,6 +106,17 @@ def close(self):

super(ProgressDialog, self).close()

# -------------------------------------------------------------------------
# 'IWidget' interface.
# -------------------------------------------------------------------------

def destroy(self):
while self._connections_to_remove:
signal, handler = self._connections_to_remove.pop()
signal.disconnect(handler)

super(ProgressDialog, self).destroy()

# -------------------------------------------------------------------------
# IProgressDialog Interface
# -------------------------------------------------------------------------
Expand Down Expand Up @@ -182,8 +202,14 @@ def _create_buttons(self, dialog, layout):

if self.can_cancel:
buttons.rejected.connect(dialog.reject)
self._connections_to_remove.append(
(buttons.rejected, dialog.reject)
)
if self.can_ok:
buttons.accepted.connect(dialog.accept)
self._connections_to_remove.append(
(buttons.accepted, dialog.accept)
)

layout.addWidget(buttons)

Expand Down

0 comments on commit 5d16083

Please sign in to comment.