From 19f1db9578b3143050efc26b9aaf6603f03ac079 Mon Sep 17 00:00:00 2001 From: Charles Haley Date: Sat, 29 Apr 2023 13:30:46 +0100 Subject: [PATCH 1/3] Fix weird problem where a menu can be shown on the wrong screen if it is constructed before the first show event. On my test system it is 100% repeatable. FWIW: I spent several hours trying to find a way to work around this issue. The trigger is clear -- populating the menu for the first time outside of the Qt "show()" process. This commit fixes it by not populating the 'real' menu until the show sequence has started. If something wants the menu before the show process has started then a fake action is returned with the menu. --- src/calibre/gui2/actions/sort.py | 67 ++++++++++++++++++++++++++++++-- 1 file changed, 64 insertions(+), 3 deletions(-) diff --git a/src/calibre/gui2/actions/sort.py b/src/calibre/gui2/actions/sort.py index 5236301bb4..36e1ef6532 100644 --- a/src/calibre/gui2/actions/sort.py +++ b/src/calibre/gui2/actions/sort.py @@ -5,10 +5,11 @@ __license__ = 'GPL v3' __copyright__ = '2013, Kovid Goyal ' from contextlib import suppress +from enum import Enum from functools import partial from qt.core import ( QAbstractItemView, QAction, QDialog, QDialogButtonBox, QIcon, QListWidget, - QListWidgetItem, QSize, Qt, QToolButton, QVBoxLayout, pyqtSignal, + QListWidgetItem, QMenu, QSize, Qt, QToolButton, QVBoxLayout, pyqtSignal, ) from calibre.gui2.actions import InterfaceAction @@ -35,6 +36,21 @@ class SortAction(QAction): self.sort_requested.emit(self.key, self.ascending) +class InitializationEnum(Enum): + WaitingInitialization = 0 + InitializeCalled = 1 + FullyInitialized = 2 + + +class FakeAction(QAction): + + def menu(self): + return self.the_menu + + def setMenu(self, menu): + self.the_menu = menu + + class SortByAction(InterfaceAction): name = 'Sort By' @@ -43,8 +59,11 @@ class SortByAction(InterfaceAction): popup_type = QToolButton.ToolButtonPopupMode.InstantPopup action_add_menu = True dont_add_to = frozenset(('context-menu-cover-browser', )) + init_complete = InitializationEnum.WaitingInitialization + menu_shown = False def genesis(self): + self.fake_action = None self.sorted_icon = QIcon.ic('ok.png') self.qaction.menu().aboutToShow.connect(self.about_to_show) @@ -57,6 +76,45 @@ class SortByAction(InterfaceAction): c('reverse_sort_action', _('Reverse current sort'), _('Reverse the current sort order'), self.reverse_sort, 'shift+f5') c('reapply_sort_action', _('Re-apply current sort'), _('Re-apply the current sort'), self.reapply_sort, 'f5') + + # The property is here because Qt can put the menu on the wrong screen if + # the menu is built for the first time outside of the process of showing it. + # On Windows this happens if calibre is on screen 2; the menu appears on + # screen 1. Why Qt does this I can't say. The problem can be triggered if a + # plugin queries the menu during its own initialization. We solve this + # problem by giving the caller a fake QAction with the menu, which works for + # plugins. We use the "real" action once the user has requested the menu. Of + # course, the right fix would be for Qt to show the menu where it should go + # even if built previously, but I can't find how to make that happen + + @property + def qaction(self): + try: + # if initialization_complete() hasn't been called or we are fully + # initialized then do nothing special + if self.init_complete == InitializationEnum.InitializeCalled: + if not self.menu_shown: + # This is the problematic case where something wants the + # menu before it's been shown. Construct the fake action + m = QMenu() + self.update_menu(m) + if self.fake_action is None: + self.fake_action = FakeAction() + self.fake_action.setMenu(m) + return self.fake_action + else: + # The menu has been shown. No need to construct the fake + # action in the future. + self.init_complete = InitializationEnum.FullyInitialized + except Exception: + import traceback + traceback.print_exc() + return self.my_qaction + + @qaction.setter + def qaction(self, val): + self.my_qaction = val + def reverse_sort(self): self.gui.current_view().reverse_sort() @@ -69,13 +127,14 @@ class SortByAction(InterfaceAction): self.menuless_qaction.setEnabled(enabled) def about_to_show(self): + self.menu_shown = True self.update_menu() def library_changed(self, db): self.update_menu() def initialization_complete(self): - self.update_menu() + self.init_complete = InitializationEnum.InitializeCalled def update_menu(self, menu=None): menu = self.qaction.menu() if menu is None else menu @@ -128,6 +187,8 @@ class SortByAction(InterfaceAction): menu.insertSeparator(before) else: menu.addAction(sac) + if self.fake_action is not None: + self.fake_action.setMenu(menu) def select_sortable_columns(self): db = self.gui.current_db @@ -166,7 +227,7 @@ class SortByAction(InterfaceAction): d = ChooseMultiSort(self.gui.current_db, parent=self.gui, is_device_connected=self.gui.device_connected) if d.exec() == QDialog.DialogCode.Accepted: self.gui.library_view.multisort(d.current_sort_spec) - self.update_menu() + self.update_menu() def sort_requested(self, key, ascending): if ascending is None: From 6f1ef2ddabba1d05ab86bf882c83397c0e5ac2de Mon Sep 17 00:00:00 2001 From: Charles Haley Date: Sun, 30 Apr 2023 10:18:25 +0100 Subject: [PATCH 2/3] Revert "Fix weird problem where a menu can be shown on the wrong screen if it is constructed before the first show event. On my test system it is 100% repeatable." This reverts commit 19f1db9578b3143050efc26b9aaf6603f03ac079. --- src/calibre/gui2/actions/sort.py | 67 ++------------------------------ 1 file changed, 3 insertions(+), 64 deletions(-) diff --git a/src/calibre/gui2/actions/sort.py b/src/calibre/gui2/actions/sort.py index 36e1ef6532..5236301bb4 100644 --- a/src/calibre/gui2/actions/sort.py +++ b/src/calibre/gui2/actions/sort.py @@ -5,11 +5,10 @@ __license__ = 'GPL v3' __copyright__ = '2013, Kovid Goyal ' from contextlib import suppress -from enum import Enum from functools import partial from qt.core import ( QAbstractItemView, QAction, QDialog, QDialogButtonBox, QIcon, QListWidget, - QListWidgetItem, QMenu, QSize, Qt, QToolButton, QVBoxLayout, pyqtSignal, + QListWidgetItem, QSize, Qt, QToolButton, QVBoxLayout, pyqtSignal, ) from calibre.gui2.actions import InterfaceAction @@ -36,21 +35,6 @@ class SortAction(QAction): self.sort_requested.emit(self.key, self.ascending) -class InitializationEnum(Enum): - WaitingInitialization = 0 - InitializeCalled = 1 - FullyInitialized = 2 - - -class FakeAction(QAction): - - def menu(self): - return self.the_menu - - def setMenu(self, menu): - self.the_menu = menu - - class SortByAction(InterfaceAction): name = 'Sort By' @@ -59,11 +43,8 @@ class SortByAction(InterfaceAction): popup_type = QToolButton.ToolButtonPopupMode.InstantPopup action_add_menu = True dont_add_to = frozenset(('context-menu-cover-browser', )) - init_complete = InitializationEnum.WaitingInitialization - menu_shown = False def genesis(self): - self.fake_action = None self.sorted_icon = QIcon.ic('ok.png') self.qaction.menu().aboutToShow.connect(self.about_to_show) @@ -76,45 +57,6 @@ class SortByAction(InterfaceAction): c('reverse_sort_action', _('Reverse current sort'), _('Reverse the current sort order'), self.reverse_sort, 'shift+f5') c('reapply_sort_action', _('Re-apply current sort'), _('Re-apply the current sort'), self.reapply_sort, 'f5') - - # The property is here because Qt can put the menu on the wrong screen if - # the menu is built for the first time outside of the process of showing it. - # On Windows this happens if calibre is on screen 2; the menu appears on - # screen 1. Why Qt does this I can't say. The problem can be triggered if a - # plugin queries the menu during its own initialization. We solve this - # problem by giving the caller a fake QAction with the menu, which works for - # plugins. We use the "real" action once the user has requested the menu. Of - # course, the right fix would be for Qt to show the menu where it should go - # even if built previously, but I can't find how to make that happen - - @property - def qaction(self): - try: - # if initialization_complete() hasn't been called or we are fully - # initialized then do nothing special - if self.init_complete == InitializationEnum.InitializeCalled: - if not self.menu_shown: - # This is the problematic case where something wants the - # menu before it's been shown. Construct the fake action - m = QMenu() - self.update_menu(m) - if self.fake_action is None: - self.fake_action = FakeAction() - self.fake_action.setMenu(m) - return self.fake_action - else: - # The menu has been shown. No need to construct the fake - # action in the future. - self.init_complete = InitializationEnum.FullyInitialized - except Exception: - import traceback - traceback.print_exc() - return self.my_qaction - - @qaction.setter - def qaction(self, val): - self.my_qaction = val - def reverse_sort(self): self.gui.current_view().reverse_sort() @@ -127,14 +69,13 @@ class SortByAction(InterfaceAction): self.menuless_qaction.setEnabled(enabled) def about_to_show(self): - self.menu_shown = True self.update_menu() def library_changed(self, db): self.update_menu() def initialization_complete(self): - self.init_complete = InitializationEnum.InitializeCalled + self.update_menu() def update_menu(self, menu=None): menu = self.qaction.menu() if menu is None else menu @@ -187,8 +128,6 @@ class SortByAction(InterfaceAction): menu.insertSeparator(before) else: menu.addAction(sac) - if self.fake_action is not None: - self.fake_action.setMenu(menu) def select_sortable_columns(self): db = self.gui.current_db @@ -227,7 +166,7 @@ class SortByAction(InterfaceAction): d = ChooseMultiSort(self.gui.current_db, parent=self.gui, is_device_connected=self.gui.device_connected) if d.exec() == QDialog.DialogCode.Accepted: self.gui.library_view.multisort(d.current_sort_spec) - self.update_menu() + self.update_menu() def sort_requested(self, key, ascending): if ascending is None: From d4d13d0f2a5002a84aacccfd963742087bdb7ea3 Mon Sep 17 00:00:00 2001 From: Charles Haley Date: Sun, 30 Apr 2023 13:08:14 +0100 Subject: [PATCH 3/3] Different fix for the wrong screen problem. --- src/calibre/gui2/actions/__init__.py | 58 ++++++++++++++++++- src/calibre/gui2/actions/manage_categories.py | 3 +- src/calibre/gui2/actions/saved_searches.py | 50 +--------------- src/calibre/gui2/actions/sort.py | 50 ++++++++++++---- 4 files changed, 98 insertions(+), 63 deletions(-) diff --git a/src/calibre/gui2/actions/__init__.py b/src/calibre/gui2/actions/__init__.py index 7543e679e0..bc38118a14 100644 --- a/src/calibre/gui2/actions/__init__.py +++ b/src/calibre/gui2/actions/__init__.py @@ -8,7 +8,7 @@ __docformat__ = 'restructuredtext en' from functools import partial from zipfile import ZipFile -from qt.core import (QToolButton, QAction, QIcon, QObject, QMenu, +from qt.core import (QToolButton, QAction, QIcon, QObject, QMenu, QPoint, QKeySequence) from calibre import prints @@ -18,6 +18,62 @@ from calibre.gui2.keyboard import NameConflict from polyglot.builtins import string_or_bytes +def toolbar_widgets_for_action(gui, action): + # Search the the toolbars for the widget associated with an action, passing + # them to the caller for further processing + for x in gui.bars_manager.bars: + try: + w = x.widgetForAction(action) + # It seems that multiple copies of the action can exist, such as + # when the device-connected menu is changed while the device is + # connected. Use the one that has an actual position. + if w is None or w.pos().x() == 0: + continue + # The button might be hidden + if not w.isVisible(): + continue + yield(w) + except Exception: + continue + + +def show_menu_under_widget(gui, menu, action, name): + # First try the tool bar + for w in toolbar_widgets_for_action(gui, action): + try: + # The w.height() assures that the menu opens below the button. + menu.exec(w.mapToGlobal(QPoint(0, w.height()))) + return + except Exception: + continue + # Now try the menu bar + for x in gui.bars_manager.menu_bar.added_actions: + # This depends on no two menus with the same name. + # I don't know if this works on a Mac + if x.text() == name: + try: + # The menu item might be hidden + if not x.isVisible(): + continue + # We can't use x.trigger() because it doesn't put the menu + # in the right place. Instead get the position of the menu + # widget on the menu bar + p = x.parent().menu_bar + r = p.actionGeometry(x) + # Make sure that the menu item is actually displayed in the menu + # and not the overflow + if p.geometry().width() < (r.x() + r.width()): + continue + # Show the menu under the name in the menu bar + menu.exec(p.mapToGlobal(QPoint(r.x()+2, r.height()-2))) + return + except Exception: + continue + # No visible button found. Fall back to displaying in upper left corner + # of the library view. + menu.exec(gui.library_view.mapToGlobal(QPoint(10, 10))) + + def menu_action_unique_name(plugin, unique_name): return '%s : menu action : %s'%(plugin.unique_name, unique_name) diff --git a/src/calibre/gui2/actions/manage_categories.py b/src/calibre/gui2/actions/manage_categories.py index 9b07cb10db..768da620b2 100644 --- a/src/calibre/gui2/actions/manage_categories.py +++ b/src/calibre/gui2/actions/manage_categories.py @@ -4,7 +4,7 @@ from qt.core import QMenu, QToolButton -from calibre.gui2.actions import InterfaceAction +from calibre.gui2.actions import InterfaceAction, show_menu_under_widget class ManageCategoriesAction(InterfaceAction): @@ -39,7 +39,6 @@ class ManageCategoriesAction(InterfaceAction): # show the menu in the upper left corner of the library view pane. Yes, this # is a bit weird but it works as well as a popping up a dialog. def show_menu(self): - from calibre.gui2.actions.saved_searches import show_menu_under_widget show_menu_under_widget(self.gui, self.menu, self.qaction, self.name) def about_to_show_menu(self): diff --git a/src/calibre/gui2/actions/saved_searches.py b/src/calibre/gui2/actions/saved_searches.py index 87bf832cc7..1d85ad274e 100644 --- a/src/calibre/gui2/actions/saved_searches.py +++ b/src/calibre/gui2/actions/saved_searches.py @@ -2,55 +2,9 @@ # License: GPLv3 Copyright: 2022, Charles Haley # -from qt.core import QPoint, QMenu, QToolButton +from qt.core import QMenu, QToolButton -from calibre.gui2.actions import InterfaceAction - - -def show_menu_under_widget(gui, menu, action, name): - # First try the tool bar - for x in gui.bars_manager.bars: - try: - w = x.widgetForAction(action) - # It seems that multiple copies of the action can exist, such as - # when the device-connected menu is changed while the device is - # connected. Use the one that has an actual position. - if w is None or w.pos().x() == 0: - continue - # The button might be hidden - if not w.isVisible(): - continue - # The w.height() assures that the menu opens below the button. - menu.exec(w.mapToGlobal(QPoint(0, w.height()))) - return - except Exception: - continue - # Now try the menu bar - for x in gui.bars_manager.menu_bar.added_actions: - # This depends on no two menus with the same name. - # I don't know if this works on a Mac - if x.text() == name: - try: - # The menu item might be hidden - if not x.isVisible(): - continue - # We can't use x.trigger() because it doesn't put the menu - # in the right place. Instead get the position of the menu - # widget on the menu bar - p = x.parent().menu_bar - r = p.actionGeometry(x) - # Make sure that the menu item is actually displayed in the menu - # and not the overflow - if p.geometry().width() < (r.x() + r.width()): - continue - # Show the menu under the name in the menu bar - menu.exec(p.mapToGlobal(QPoint(r.x()+2, r.height()-2))) - return - except Exception: - continue - # No visible button found. Fall back to displaying in upper left corner - # of the library view. - menu.exec(gui.library_view.mapToGlobal(QPoint(10, 10))) +from calibre.gui2.actions import InterfaceAction, show_menu_under_widget class SavedSearchesAction(InterfaceAction): diff --git a/src/calibre/gui2/actions/sort.py b/src/calibre/gui2/actions/sort.py index 5236301bb4..fe104ee2d7 100644 --- a/src/calibre/gui2/actions/sort.py +++ b/src/calibre/gui2/actions/sort.py @@ -8,10 +8,10 @@ from contextlib import suppress from functools import partial from qt.core import ( QAbstractItemView, QAction, QDialog, QDialogButtonBox, QIcon, QListWidget, - QListWidgetItem, QSize, Qt, QToolButton, QVBoxLayout, pyqtSignal, + QListWidgetItem, QMenu, QSize, Qt, QToolButton, QVBoxLayout, pyqtSignal, ) -from calibre.gui2.actions import InterfaceAction +from calibre.gui2.actions import InterfaceAction, show_menu_under_widget, toolbar_widgets_for_action from calibre.library.field_metadata import category_icon_map from calibre.utils.icu import primary_sort_key from polyglot.builtins import iteritems @@ -40,13 +40,25 @@ class SortByAction(InterfaceAction): name = 'Sort By' action_spec = (_('Sort by'), 'sort.png', _('Sort the list of books'), None) action_type = 'current' - popup_type = QToolButton.ToolButtonPopupMode.InstantPopup + popup_type = QToolButton.ToolButtonPopupMode.MenuButtonPopup action_add_menu = True dont_add_to = frozenset(('context-menu-cover-browser', )) def genesis(self): self.sorted_icon = QIcon.ic('ok.png') - self.qaction.menu().aboutToShow.connect(self.about_to_show) + self.qaction.triggered.connect(self.show_menu) + + # Create a "hidden" menu that can have a shortcut. This also lets us + # manually show the menu instead of letting Qt do it to work around a + # problem where Qt can show the menu on the wrong screen. + self.hidden_menu = QMenu() + self.shortcut_action = self.create_menu_action( + menu=self.hidden_menu, + unique_name=_('Sort by'), + text=_('Show the Sort by menu'), + icon=None, + shortcut='Ctrl+F5', + triggered=self.show_menu) def c(attr, title, tooltip, callback, keys=()): ac = self.create_action(spec=(title, None, tooltip, keys), attr=attr) @@ -57,6 +69,12 @@ class SortByAction(InterfaceAction): c('reverse_sort_action', _('Reverse current sort'), _('Reverse the current sort order'), self.reverse_sort, 'shift+f5') c('reapply_sort_action', _('Re-apply current sort'), _('Re-apply the current sort'), self.reapply_sort, 'f5') + def show_menu(self): + # We manually show the menu instead of letting Qt do it to work around a + # problem where the menu can show on the wrong screen. + self.update_menu() + show_menu_under_widget(self.gui, self.qaction.menu(), self.qaction, self.name) + def reverse_sort(self): self.gui.current_view().reverse_sort() @@ -68,17 +86,20 @@ class SortByAction(InterfaceAction): self.qaction.setEnabled(enabled) self.menuless_qaction.setEnabled(enabled) - def about_to_show(self): - self.update_menu() - def library_changed(self, db): self.update_menu() def initialization_complete(self): self.update_menu() + # Remove the down arrow from the buttons as they serve no purpose. They + # show exactly what clicking the button shows + for w in toolbar_widgets_for_action(self.gui, self.qaction): + # Not sure why both styles are necessary, but they are + w.setStyleSheet('QToolButton::menu-button {image: none; }' + 'QToolButton::menu-arrow {image: none; }') - def update_menu(self, menu=None): - menu = self.qaction.menu() if menu is None else menu + def update_menu(self): + menu = self.qaction.menu() for action in menu.actions(): if hasattr(action, 'sort_requested'): action.sort_requested.disconnect() @@ -96,17 +117,22 @@ class SortByAction(InterfaceAction): menu.addAction(name, partial(self.named_sort_selected, saved_sorts[name])) menu.addSeparator() + # Note the current sort column so it can be specially handled below try: sort_col, order = m.sorted_on except TypeError: sort_col, order = 'date', True - fm = db.field_metadata - name_map = {v:k for k, v in iteritems(fm.ui_sortable_field_keys())} - hidden = frozenset(db.new_api.pref(SORT_HIDDEN_PREF, default=()) or ()) + + # The operations to choose which columns to display and to create saved sorts menu.addAction(_('Select sortable columns')).triggered.connect(self.select_sortable_columns) menu.addAction(_('Sort on multiple columns'), self.choose_multisort) menu.addSeparator() + + # Add the columns to the menu + fm = db.field_metadata + name_map = {v:k for k, v in iteritems(fm.ui_sortable_field_keys())} all_names = sorted(name_map, key=primary_sort_key) + hidden = frozenset(db.new_api.pref(SORT_HIDDEN_PREF, default=()) or ()) for name in all_names: key = name_map[name]