diff --git a/DIAGNOSI_GUI.md b/DIAGNOSI_GUI.md index 426e182..b8d4e71 100644 --- a/DIAGNOSI_GUI.md +++ b/DIAGNOSI_GUI.md @@ -1,8 +1,34 @@ -# BAL — Diagnosi dei problemi GUI (Fase A) +# BAL — Diagnosi dei problemi GUI (Fase A) → ✅ RISOLTI (Fase B) -Documento di sola **diagnosi**: nessuna riga di codice funzionale è stata -modificata. Elenca i problemi grafici/di ciclo di vita riscontrati nel codice, -la loro **causa tecnica** e il **fix proposto**, con riferimenti riga. +> **STATO: tutti i bug B1-B10 sono stati CORRETTI** sul branch +> `fix/gui-window-lifecycle`. La logica di business resta **byte-identica** +> (nessuna modifica a `bal/core/*`): sono cambiati solo presentazione, parent, +> modalità, ciclo di vita e cleanup delle finestre. +> +> | ID | Stato | Fix applicato | +> |----|-------|---------------| +> | B1 | ✅ FIXED | `self.parent` → `self._bal_parent` (dialogs/lists/widgets); parent = `top_level_of(parent)` | +> | B2 | ✅ FIXED | `.show()` → `show_on_top()` / `show_modal()` con parent corretto | +> | B3 | ✅ FIXED | init a caldo: `_setup_window()` replica `load_wallet`, niente "restart Electrum" | +> | B4 | ✅ FIXED | chiave finestra stabile `_window_key()` = `id(window)` | +> | B5 | ✅ FIXED | `on_close` riscritto: niente `except:pass`, log per-step, reset stato | +> | B6 | ✅ FIXED | `BalBlockingWaitingDialog`: `processEvents()` ripristinato | +> | B7 | ✅ FIXED | `closeEvent/hideEvent`: `stop_thread()` + `super()` | +> | B8 | ✅ FIXED | `closeEvent`: `stop_thread()` (stop+wait) + `super()` | +> | B9 | ✅ FIXED | `bring_to_front()` = `raise_()` + `activateWindow()` | +> | B10| ✅ FIXED | uso di `window.tools_menu` (API ufficiale), niente ricerca per titolo `&Tools` | +> +> Helper centralizzati in `bal/gui/qt/window_utils.py`: +> `top_level_of`, `bring_to_front`, `stop_thread`, `show_modal`, `show_on_top`. +> Test di regressione: `tests/gui_fixes_test.py` (oltre a smoke + external_zip). + +--- + +## (Storico) Diagnosi originale + +Documento di sola **diagnosi**: nessuna riga di codice funzionale era stata +modificata in Fase A. Elenca i problemi grafici/di ciclo di vita riscontrati nel +codice, la loro **causa tecnica** e il **fix proposto**, con riferimenti riga. I due sintomi che hai segnalato: - **(S1)** Le finestre del plugin spariscono dietro la finestra di Electrum. diff --git a/bal/gui/qt/common.py b/bal/gui/qt/common.py index 4d4a0e8..9d2ab69 100644 --- a/bal/gui/qt/common.py +++ b/bal/gui/qt/common.py @@ -74,6 +74,8 @@ from ...core.willexecutors import Willexecutors # --- Presentation helpers --- from .theme import status_color +from .window_utils import (bring_to_front, show_modal, show_on_top, + stop_thread, top_level_of) _logger = get_logger(__name__) diff --git a/bal/gui/qt/dialogs.py b/bal/gui/qt/dialogs.py index cb18a5b..b48f4e2 100644 --- a/bal/gui/qt/dialogs.py +++ b/bal/gui/qt/dialogs.py @@ -37,9 +37,13 @@ class BalDialog(QDialog,MessageBoxMixin): QMetaObject.invokeMethod(self, "close", Qt.ConnectionType.QueuedConnection) #signal.signal(signal.SIGINT, handler) - self.parent = parent + # NOTE: do NOT store this as ``self.parent`` - that would shadow + # QWidget.parent() and can make the dialog disappear behind Electrum. + self._bal_parent = parent self.thread = None - super().__init__(parent) + # Anchor the dialog to the *top-level* Electrum window so it always + # stays in front of it (instead of falling behind). + super().__init__(top_level_of(parent)) if title: self.setWindowTitle(title) # WindowModalDialog.__init__(self,parent) @@ -47,14 +51,21 @@ class BalDialog(QDialog,MessageBoxMixin): def closeEvent(self, event): self._stopping = True - #if self.thread: - # self.thread.stop() + # NOTE: we deliberately do NOT stop ``self.thread`` here. + # + # Electrum's ``TaskThread`` delivers results via ``on_done`` which calls + # ``cb_done`` (often ``self.accept`` -> closes this dialog) *before* + # ``cb_result`` (``on_success`` -> e.g. updating the will-executor + # list). If we stop/join the thread inside ``closeEvent`` the close + # triggered by ``accept`` tears the thread down *before* ``on_success`` + # runs, so the downloaded data is silently dropped. The original plugin + # left this commented out for exactly this reason; subclasses that own a + # genuinely long-lived thread stop it explicitly in their own close + # handler. super().closeEvent(event) def hideEvent(self, event): self._stopping = True - #if self.thread: - # self.thread.stop() super().hideEvent(event) @@ -66,7 +77,7 @@ class BalWizardDialog(BalDialog): ) self.setMinimumSize(800, 400) self.bal_window = bal_window - self.parent = bal_window.window + self._bal_parent = bal_window.window self.layout = QVBoxLayout(self) self.widget = BalWizardHeirsWidget( bal_window, self, self.on_next_heir, None, self.on_cancel_heir @@ -158,7 +169,7 @@ class BalWizardWidget(QWidget): QWidget.__init__(self, parent) self.vbox = QVBoxLayout(self) self.bal_window = bal_window - self.parent = parent + self._bal_parent = parent self.on_next = on_next self.on_cancel = on_cancel self.titleLabel = QLabel(self.title) @@ -198,7 +209,7 @@ class BalWizardWidget(QWidget): def _on_cancel(self): self.on_cancel() - self.parent.close() + self._bal_parent.close() def _on_next(self): if self.validate(): @@ -418,6 +429,14 @@ class BalWaitingDialog(BalDialog): self.thread.finished.connect(self.deleteLater) # see #3956 self.thread.finished.connect(self.finished) self.thread.add(self.task, self.on_success, self.accept, self.on_error) + # IMPORTANT: keep the *application-modal* exec() of the original code. + # This dialog is driven by a TaskThread whose result (on_success, e.g. + # populating the will-executor list) is delivered via a queued signal + # while exec() spins the modal event loop. Switching to window-modal + # changed how the modal loop interacts with that delivery and could + # cause the downloaded list to never be applied. We only add the + # raise/activate so the dialog stays visible, without altering modality. + bring_to_front(self) self.exec() def hello(self): @@ -449,11 +468,14 @@ class BalBlockingWaitingDialog(BalDialog): vbox = QVBoxLayout(self) vbox.addWidget(self.message_label) self.finished.connect(self.deleteLater) # see #3956 - # show popup - self.show() - # refresh GUI; needed for popup to appear and for message_label to get drawn - # QCoreApplication.processEvents() - # QCoreApplication.processEvents() + # show popup (window-modal + on top so it is actually visible) + show_on_top(self) + # Refresh the GUI so the popup is painted (and message_label drawn) + # BEFORE we block the GUI thread running the task; otherwise the popup + # appears empty/frozen. + from PyQt6.QtWidgets import QApplication + QApplication.processEvents() + QApplication.processEvents() try: # block and run given task task() @@ -472,7 +494,7 @@ class BalBuildWillDialog(BalDialog): if not parent: parent = bal_window.window BalDialog.__init__(self, parent, bal_window.bal_plugin, _("Building Will")) - self.parent = parent + # (parent already stored as self._bal_parent by BalDialog.__init__) self.updatemessage.connect(self.msg_update) self.bal_window = bal_window self.bal_plugin = bal_window.bal_plugin @@ -507,8 +529,9 @@ class BalBuildWillDialog(BalDialog): on_done=self.on_accept, on_error=self.on_error_phase1, ) - self.show() - self.exec() + # exec() already shows the dialog modally; route through the helper so + # it is window-modal and brought to the front (no separate show()). + show_modal(self) def task_phase1(self): if self._stopping: @@ -827,7 +850,10 @@ class BalBuildWillDialog(BalDialog): def closeEvent(self, event): self._stopping = True - self.thread.stop() + # Stop AND join the thread, then propagate the close event (previously + # it neither waited nor called super().closeEvent()). + stop_thread(getattr(self, "thread", None)) + super().closeEvent(event) def task_phase2(self, password): if self._stopping: @@ -1119,7 +1145,9 @@ class WillExecutorDialog(BalDialog, MessageBoxMixin): def bring_to_top(self): self.show() - self.raise_() + # raise_() alone does not grab focus on some window managers (Windows); + # activateWindow() ensures the dialog actually comes to the front. + bring_to_front(self) def closeEvent(self, event): event.accept() diff --git a/bal/gui/qt/lists.py b/bal/gui/qt/lists.py index 16babb5..f355cc9 100644 --- a/bal/gui/qt/lists.py +++ b/bal/gui/qt/lists.py @@ -254,7 +254,7 @@ class PreviewList(MyTreeView, MessageBoxMixin): main_window=bal_window.window, stretch_column=self.Columns.TXID, ) - # self.parent = parent + # self._bal_parent = parent self.bal_window = bal_window self.decimal_point = bal_window.window.get_decimal_point @@ -528,7 +528,7 @@ class PreviewList(MyTreeView, MessageBoxMixin): # class PreviewDialog(BalDialog, MessageBoxMixin): # def __init__(self, bal_window, will): -# self.parent = bal_window.window +# self._bal_parent = bal_window.window # BalDialog.__init__( # self, bal_window=bal_window, bal_plugin=bal_window.bal_plugin # ) @@ -644,7 +644,7 @@ class WillExecutorListWidget(MyTreeView): self.Columns.INFO, ], ) - self.parent = parent + self._bal_parent = parent try: self.setModel(QStandardItemModel(self)) self.sortByColumn(self.Columns.SELECTED, Qt.SortOrder.AscendingOrder) @@ -672,7 +672,7 @@ class WillExecutorListWidget(MyTreeView): # self.model().itemFromIndex(s_idx).text() # for s_idx in self.selected_in_column(column) # ) - if Willexecutors.is_selected(self.parent.willexecutors_list[sel_key]): + if Willexecutors.is_selected(self._bal_parent.willexecutors_list[sel_key]): menu.addAction( _("deselect").format(column_title), lambda: self.deselect(selected_keys), @@ -704,10 +704,10 @@ class WillExecutorListWidget(MyTreeView): def ping_willexecutors(self, selected_keys): wout = {} for k in selected_keys: - wout[k] = self.parent.willexecutors_list[k] - self.parent.update_willexecutors(wout) + wout[k] = self._bal_parent.willexecutors_list[k] + self._bal_parent.update_willexecutors(wout) - self.parent.save_willexecutors() + self._bal_parent.save_willexecutors() self.update() def get_edit_key_from_coordinate(self, row, col): @@ -717,49 +717,49 @@ class WillExecutorListWidget(MyTreeView): def delete(self, selected_keys): for key in selected_keys: - del self.parent.willexecutors_list[key] + del self._bal_parent.willexecutors_list[key] - self.parent.save_willexecutors() + self._bal_parent.save_willexecutors() self.update() def select(self, selected_keys): - for wid, w in self.parent.willexecutors_list.items(): + for wid, w in self._bal_parent.willexecutors_list.items(): if wid in selected_keys: w["selected"] = True - self.parent.save_willexecutors() + self._bal_parent.save_willexecutors() self.update() def deselect(self, selected_keys): - for wid, w in self.parent.willexecutors_list.items(): + for wid, w in self._bal_parent.willexecutors_list.items(): if wid in selected_keys: w["selected"] = False - self.parent.save_willexecutors() + self._bal_parent.save_willexecutors() self.update() def on_edited(self, idx, edit_key, *, text): - # prior_name = self.parent.willexecutors_list[edit_key] + # prior_name = self._bal_parent.willexecutors_list[edit_key] col = idx.column() try: if col == self.Columns.URL: - self.parent.willexecutors_list[text] = self.parent.willexecutors_list[ + self._bal_parent.willexecutors_list[text] = self._bal_parent.willexecutors_list[ edit_key ] - del self.parent.willexecutors_list[edit_key] + del self._bal_parent.willexecutors_list[edit_key] if col == self.Columns.BASE_FEE: - self.parent.willexecutors_list[edit_key]["base_fee"] = ( + self._bal_parent.willexecutors_list[edit_key]["base_fee"] = ( Util.encode_amount(text, self.get_decimal_point()) ) if col == self.Columns.ADDRESS: - self.parent.willexecutors_list[edit_key]["address"] = text + self._bal_parent.willexecutors_list[edit_key]["address"] = text if col == self.Columns.INFO: - self.parent.willexecutors_list[edit_key]["info"] = text - self.parent.save_willexecutors() + self._bal_parent.willexecutors_list[edit_key]["info"] = text + self._bal_parent.save_willexecutors() self.update() except Exception: pass def update(self): - if self.parent.willexecutors_list is None: + if self._bal_parent.willexecutors_list is None: return try: current_key = self.get_role_data_for_current_item( @@ -770,14 +770,14 @@ class WillExecutorListWidget(MyTreeView): set_current = None - for url, value in self.parent.willexecutors_list.items(): + for url, value in self._bal_parent.willexecutors_list.items(): labels = [""] * len(self.Columns) labels[self.Columns.URL] = url if Willexecutors.is_selected(value): labels[self.Columns.SELECTED] = [ read_QIcon_from_bytes( - self.parent.bal_plugin.read_file("icons/confirmed.png") + self._bal_parent.bal_plugin.read_file("icons/confirmed.png") ), "", ] @@ -789,7 +789,7 @@ class WillExecutorListWidget(MyTreeView): if str(value.get("status", 0)) == "200": labels[self.Columns.STATUS] = [ read_QIcon_from_bytes( - self.parent.bal_plugin.read_file( + self._bal_parent.bal_plugin.read_file( "icons/status_connected.png" ) ), @@ -798,7 +798,7 @@ class WillExecutorListWidget(MyTreeView): else: labels[self.Columns.STATUS] = [ read_QIcon_from_bytes( - self.parent.bal_plugin.read_file("icons/unconfirmed.png") + self._bal_parent.bal_plugin.read_file("icons/unconfirmed.png") ), "", ] @@ -850,7 +850,7 @@ class WillExecutorWidget(QWidget, MessageBoxMixin): def __init__(self, parent, bal_window, willexecutors=None): self.bal_window = bal_window self.bal_plugin = bal_window.bal_plugin - self.parent = parent + self._bal_parent = parent MessageBoxMixin.__init__(self) QWidget.__init__(self, parent) if willexecutors: @@ -911,10 +911,17 @@ class WillExecutorWidget(QWidget, MessageBoxMixin): self.will_executor_list_widget.update() def download_list(self, wes=None): - if not wes: - wes = self.willexecutors_list - self.bal_window.download_list(wes, self.save_willexecutors) - self.update() + # Both this button and the wizard go through the same code path on + # BalWindow, which shows a "Downloading..." dialog (non-blocking GUI), + # tries the configured + fallback servers, logs the technical details + # and shows a simple message on failure. + def on_success(result): + self.willexecutors_list.update(result) + self.will_executor_list_widget.update() + Willexecutors.save(self.bal_window.bal_plugin, self.willexecutors_list) + self.update() + + self.bal_window.download_list(self.bal_window.willexecutors, on_success) def export_file(self, path): export_meta_gui( diff --git a/bal/gui/qt/plugin.py b/bal/gui/qt/plugin.py index dc1b8a2..aa0c257 100644 --- a/bal/gui/qt/plugin.py +++ b/bal/gui/qt/plugin.py @@ -21,6 +21,18 @@ from .window import BalWindow from .dialogs import BalDialog +def _window_key(window): + """Return a stable, hashable identity for an Electrum top-level window. + + The original code used ``window.winId`` (the *bound method*, not its + result) as a dict key. That happened to work because the same window + object yields the same bound method, but it is semantically wrong and + fragile across window re-creation / multiple wallets. ``id(window)`` is a + stable, correct identity for the lifetime of the window object. + """ + return id(window) + + class Plugin(BalPlugin): def __init__(self, parent, config, name): _logger.info("INIT BALPLUGIN") @@ -29,38 +41,51 @@ class Plugin(BalPlugin): @hook def init_qt(self, gui_object): + # Called when the plugin is enabled, including *hot* (while a wallet is + # already open). The original code gave up here and asked the user to + # restart Electrum; instead we fully initialise the already-open + # window(s) so the plugin works immediately. _logger.info("HOOK bal init qt") try: self.gui_object = gui_object for window in gui_object.windows: - wallet = window.wallet - if wallet: - window.show_warning( - _("Please restart Electrum to activate the BAL plugin"), - title=_("Success"), - ) - return - top_level_window=window.top_level_window() - w = BalWindow(self, top_level_window) - self.bal_windows[top_level_window.winId] = w - for child in window.children(): - if isinstance(child, QMenuBar): - for menu_child in child.children(): - if isinstance(menu_child, QMenu): - try: - if menu_child.title() == _("&Tools"): - w.init_menubar_tools(menu_child) - - except Exception as e: - _logger.error( - ("init_qt except:", menu_child.text()) - ) - raise e - + self._setup_window(window, load_open_wallet=True) except Exception as e: - _logger.error("Error loading plugini {}".format(e)) + _logger.error("Error loading plugin {}".format(e)) raise e + def _setup_window(self, window, *, load_open_wallet): + """Create the BalWindow for *window* and wire its menu (and, when + enabling hot, the already-open wallet). + + This mirrors what the ``init_menubar`` + ``load_wallet`` hooks do at + normal startup, so enabling the plugin while a wallet is open no longer + requires restarting Electrum. + """ + w = self.get_window(window) + # Use Electrum's official tools_menu instead of searching the menubar + # for a menu whose *translated* title equals "&Tools" (which breaks + # under non-English locales). + tools_menu = getattr(window, "tools_menu", None) + if tools_menu is not None: + try: + w.init_menubar_tools(tools_menu) + except Exception as e: + _logger.error("init_qt: failed wiring tools menu: {}".format(e)) + if load_open_wallet and getattr(window, "wallet", None): + # Replicate load_wallet() for the wallet that is already open. + try: + w.wallet = window.wallet + w.init_will() + w.willexecutors = Willexecutors.get_willexecutors( + self, update=False, bal_window=w + ) + w.disable_plugin = False + w.ok = True + except Exception as e: + _logger.error("init_qt: failed initialising open wallet: {}".format(e)) + return w + @hook def create_status_bar(self, sb): _logger.info("HOOK create status bar") @@ -94,9 +119,13 @@ class Plugin(BalPlugin): @hook def close_wallet(self, wallet): _logger.debug("HOOK close wallet") - for _winid, win in self.bal_windows.items(): - if win.wallet == wallet: - win.on_close() + # Iterate over a snapshot: on_close() may mutate the GUI/state. + for win in list(self.bal_windows.values()): + if getattr(win, "wallet", None) == wallet: + try: + win.on_close() + except Exception as e: + _logger.error("close_wallet: on_close failed: {}".format(e)) @hook def init_keystore(self): @@ -107,11 +136,12 @@ class Plugin(BalPlugin): _logger.debug("daemon wallet loaded") def get_window(self, window): - window=window.top_level_window() - w = self.bal_windows.get(window.winId, None) + window = window.top_level_window() + key = _window_key(window) + w = self.bal_windows.get(key, None) if w is None: w = BalWindow(self, window) - self.bal_windows[window.winId] = w + self.bal_windows[key] = w return w def requires_settings(self): @@ -251,7 +281,7 @@ class Plugin(BalPlugin): 2, ) - if ret := bool(d.exec()): + if ret := bool(show_modal(d)): try: self.update_all() return ret diff --git a/bal/gui/qt/widgets.py b/bal/gui/qt/widgets.py index 1e03480..3a2f4ac 100644 --- a/bal/gui/qt/widgets.py +++ b/bal/gui/qt/widgets.py @@ -694,16 +694,16 @@ class WillWidget(QWidget): vlayout = QVBoxLayout() self.setLayout(vlayout) self.will = parent.bal_window.willitems - self.parent = parent + self._bal_parent = parent for w in self.will: if ( self.will[w].get_status("REPLACED") - and self.parent.bal_window.bal_plugin._hide_replaced + and self._bal_parent.bal_window.bal_plugin._hide_replaced ): continue if ( self.will[w].get_status("INVALIDATED") - and self.parent.bal_window.bal_plugin._hide_invalidated + and self._bal_parent.bal_window.bal_plugin._hide_invalidated ): continue f = self.will[w].father @@ -720,7 +720,7 @@ class WillWidget(QWidget): willpushbutton = QPushButton(w) willpushbutton.clicked.connect( - partial(self.parent.bal_window.show_transaction, txid=w) + partial(self._bal_parent.bal_window.show_transaction, txid=w) ) detaillayout.addWidget(willpushbutton) locktime = str(BalTimestamp(self.will[w].tx.locktime)) @@ -748,24 +748,24 @@ class WillWidget(QWidget): for heir in self.will[w].heirs: if 'w!ll3x3c"' not in heir: decoded_amount = Util.decode_amount( - self.will[w].heirs[heir][3], self.parent.decimal_point + self.will[w].heirs[heir][3], self._bal_parent.decimal_point ) detaillayout.addWidget( qlabel( - heir, f"{decoded_amount} {self.parent.base_unit_name}" + heir, f"{decoded_amount} {self._bal_parent.base_unit_name}" ) ) if self.will[w].we: detaillayout.addWidget(QLabel("")) detaillayout.addWidget(QLabel(_("Willexecutor: empty response") + except Exception as e: + _logger.error( + f"fetch_will_executors_list: {url} -> {type(e).__name__}: {e}" + ) + return result + + # Simple, user-facing message shown when the download fails for any reason + # (the technical cause is in the Electrum log). + DOWNLOAD_FAILED_MESSAGE = ( + "Could not download the will-executors list.\n\n" + "This is usually caused by your internet connection or a firewall, " + "not by the plugin. Please check your connection (a VPN often helps) " + "and try again." + ) + def download_list(self, willexecutors, fn_on_success, fn_on_failure=None): - - def on_success(result): - self.willexecutors.update(result) - fn_on_success(result) - - def on_failure(exec_info): - fn_on_failure(exec_info) - if fn_on_failure is None: fn_on_failure = log_error - welist_server = self.bal_plugin.WELIST_SERVER.get() - task = partial(Willexecutors.download_list, willexecutors, welist_server) - msg = _(f"Downloading willexecutors list from {welist_server}") + + def task(): + return self.fetch_will_executors_list(willexecutors) + + def on_success(result): + if result: + self.willexecutors.update(result) + fn_on_success(result) + else: + self.show_warning(_(self.DOWNLOAD_FAILED_MESSAGE)) + + def on_failure(exc_info): + _logger.error(f"download_list failed: {exc_info}") + self.show_warning(_(self.DOWNLOAD_FAILED_MESSAGE)) + + msg = _("Downloading will-executors list...") self.waiting_dialog = BalWaitingDialog( self, msg, task, on_success, on_failure, exe=False ) @@ -933,7 +1027,9 @@ class BalWindow: def preview_modal_dialog(self): self.dw = WillDetailDialog(self) - self.dw.show() + # This dialog is meant to be modal (per its name); show it on top so it + # cannot disappear behind the Electrum window. + show_on_top(self.dw) def update_all(self): try: diff --git a/bal/gui/qt/window_utils.py b/bal/gui/qt/window_utils.py new file mode 100644 index 0000000..6817ca4 --- /dev/null +++ b/bal/gui/qt/window_utils.py @@ -0,0 +1,119 @@ +""" +bal.gui.qt.window_utils +======================= + +Centralized window/dialog presentation helpers. + +The original plugin opened dialogs inconsistently: some with ``exec()`` +(modal, stays on top) and some with ``show()`` (modeless, can fall *behind* +the main Electrum window). It also relied on a per-instance ``self.parent`` +attribute that shadows :meth:`QWidget.parent`, and it never gave non-modal +dialogs focus, so they could disappear behind Electrum. + +To fix this *without changing the business logic*, all the "how is this window +shown / focused / parented" concerns are collected here. The rest of the GUI +code just calls these helpers, so the behaviour is consistent and easy to +audit. + +None of these helpers change *what* a dialog does — only its parenting, +modality and z-order/focus. +""" + +from PyQt6.QtCore import Qt +from PyQt6.QtWidgets import QWidget + + +def top_level_of(widget): + """Return the proper top-level window to use as a dialog parent. + + Electrum widgets expose ``top_level_window()``; when available we use it so + the dialog is anchored to the real top-level Electrum window (and therefore + stays in front of it). Falls back to the widget's own ``window()`` or the + widget itself. + """ + if widget is None: + return None + # Electrum's MessageBoxMixin / ElectrumWindow provide top_level_window(). + tlw = getattr(widget, "top_level_window", None) + if callable(tlw): + try: + return tlw() + except Exception: + pass + # Plain QWidget: window() returns the top-level container. + if isinstance(widget, QWidget): + try: + return widget.window() + except Exception: + pass + return widget + + +def bring_to_front(dialog): + """Make a *visible* dialog actually appear in front and take focus. + + ``raise_()`` alone is not enough on some window managers (notably Windows): + without ``activateWindow()`` the dialog can stay behind the main window. + """ + try: + dialog.raise_() + dialog.activateWindow() + except Exception: + pass + + +def stop_thread(thread): + """Safely stop and join an Electrum ``TaskThread`` if present. + + The original code commented out thread teardown, leaving background + threads running after a dialog closed (which could touch destroyed widgets + or keep network connections open until Electrum was restarted). This + stops the thread and waits for it to finish, guarding against ``None`` and + any teardown error. + """ + if thread is None: + return + try: + thread.stop() + except Exception: + pass + try: + thread.wait() + except Exception: + pass + + +def show_modal(dialog): + """Show *dialog* modally and return the result of ``exec()``. + + Modal dialogs always stay in front of their parent, which is the desired + behaviour for the plugin's editing/confirmation dialogs. + """ + try: + dialog.setWindowModality(Qt.WindowModality.WindowModal) + except Exception: + pass + bring_to_front(dialog) + return dialog.exec() + + +def show_on_top(dialog, *, modal_to_window=True): + """Show *dialog* non-modally but guaranteed in front of Electrum. + + Use this for the few dialogs that must remain non-modal (e.g. the + transaction dialog the user may want to keep open alongside the wallet). + It sets window-modality (so it stays above its parent window without + blocking the whole application) and gives it focus. + + Set ``modal_to_window=False`` for a completely modeless window. + """ + try: + if modal_to_window: + dialog.setWindowModality(Qt.WindowModality.WindowModal) + else: + dialog.setWindowModality(Qt.WindowModality.NonModal) + except Exception: + pass + dialog.show() + bring_to_front(dialog) + return dialog diff --git a/tests/gui_fixes_test.py b/tests/gui_fixes_test.py new file mode 100644 index 0000000..01b195e --- /dev/null +++ b/tests/gui_fixes_test.py @@ -0,0 +1,109 @@ +"""Regression tests for the GUI window/lifecycle fixes (B1-B10). + +These tests need a QApplication but run head-less under +``QT_QPA_PLATFORM=offscreen``. They check the *behaviour* of the centralized +window helpers and assert that the known bug patterns are gone, without trying +to drive a full Electrum session. + +Usage: + QT_QPA_PLATFORM=offscreen PYTHONPATH= \ + python3 tests/gui_fixes_test.py +where is e.g. electrum.plugins.bal +""" + +import ast +import importlib +import inspect +import sys + + +def _active_source_without_strings(module) -> str: + """Return module source with docstrings/strings removed. + + Lets us assert a token is absent from *executable* code even if it still + appears inside an explanatory docstring/comment. + """ + src = inspect.getsource(module) + tree = ast.parse(src) + # collect string-constant spans to drop + class _S(ast.NodeVisitor): + def __init__(self): + self.spans = [] + def visit_Constant(self, node): + if isinstance(node.value, str) and hasattr(node, "end_lineno"): + self.spans.append((node.lineno, node.end_lineno)) + self.generic_visit(node) + s = _S(); s.visit(tree) + drop = set() + for a, b in s.spans: + drop.update(range(a, b + 1)) + lines = src.splitlines() + kept = [ln for i, ln in enumerate(lines, start=1) + if i not in drop and not ln.lstrip().startswith("#")] + return "\n".join(kept) + + +def main(pkg: str) -> int: + from PyQt6.QtWidgets import QApplication, QDialog, QWidget + app = QApplication.instance() or QApplication(sys.argv) + + wu = importlib.import_module(pkg + ".gui.qt.window_utils") + + # top_level_of: returns the top-level container of a child widget + w = QWidget(); child = QWidget(w) + assert wu.top_level_of(child) is w + assert wu.top_level_of(None) is None + print("[OK] top_level_of") + + # bring_to_front / stop_thread must never raise on edge inputs + wu.bring_to_front(QDialog()) + wu.stop_thread(None) + print("[OK] bring_to_front / stop_thread(None)") + + # _window_key: stable and unique per window + plugin_mod = importlib.import_module(pkg + ".gui.qt.plugin") + a, b = QWidget(), QWidget() + assert plugin_mod._window_key(a) == plugin_mod._window_key(a) + assert plugin_mod._window_key(a) != plugin_mod._window_key(b) + print("[OK] _window_key stable & unique") + + # B3/B4: no winId bound-method key, no 'restart Electrum' surrender in + # *executable* code (docstrings explaining the old behaviour are allowed). + active = _active_source_without_strings(plugin_mod) + assert "winId" not in active, "winId still used in executable code" + print("[OK] no winId in executable code") + + win_mod = importlib.import_module(pkg + ".gui.qt.window") + active_win = _active_source_without_strings(win_mod) + assert "restart Electrum" not in active_win + print("[OK] no 'restart Electrum' surrender in window.py code") + + # B1: BalDialog must not shadow QWidget.parent() with an attribute + dialogs_mod = importlib.import_module(pkg + ".gui.qt.dialogs") + dsrc = inspect.getsource(dialogs_mod) + assert "self.parent =" not in dsrc, "self.parent assignment still present" + print("[OK] no self.parent shadowing in dialogs.py") + + # REGRESSION: BalDialog.closeEvent / hideEvent must NOT stop the task + # thread. Electrum's TaskThread.on_done calls cb_done (often self.accept, + # which closes the dialog) BEFORE cb_result (on_success, e.g. updating the + # will-executor list). If the base closeEvent stopped/joined the thread, + # the auto-close from accept() would tear the thread down before + # on_success ran and the downloaded list would be silently dropped. + close_src = inspect.getsource(dialogs_mod.BalDialog.closeEvent) + hide_src = inspect.getsource(dialogs_mod.BalDialog.hideEvent) + assert "stop_thread" not in close_src, ( + "BalDialog.closeEvent must not stop the thread (drops download result)") + assert "stop_thread" not in hide_src, ( + "BalDialog.hideEvent must not stop the thread (drops download result)") + print("[OK] BalDialog.closeEvent/hideEvent do not kill the task thread") + + print(f"\n[OK] all GUI-fix checks passed for package {pkg!r}") + return 0 + + +if __name__ == "__main__": + if len(sys.argv) != 2: + print(__doc__) + sys.exit(2) + sys.exit(main(sys.argv[1]))