Bug 382338 - Mac: sortedDocumentList menu doesn't change focus
Summary: Mac: sortedDocumentList menu doesn't change focus
Status: RESOLVED FIXED
Alias: None
Product: kdevplatform
Classification: Developer tools
Component: sublime (show other bugs)
Version: git master
Platform: Compiled Sources macOS
: NOR normal
Target Milestone: ---
Assignee: kdevelop-bugs-null
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2017-07-14 12:09 UTC by RJVB
Modified: 2017-08-07 18:03 UTC (History)
1 user (show)

See Also:
Latest Commit:
Version Fixed In: 5.1.2
Sentry Crash Report:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description RJVB 2017-07-14 12:09:41 UTC
I'm seeing an issue where selecting a document in the dropdown sorted list of open documents changes the document view but doesn't set the focus and window title accordingly. One needs to click in the editor view to finalise the operation. Probably not a big deal if you want to edit the document but a lot more annoying when you are cleaning house and just want to close the document using the keyboard shortcut.

This happens with both the Cocoa and the XCB QPAs, even when displaying remotely on a Linux system (by contrast, it does not happen when running Kdevelop from a remote Linux system connected to XQuartz).

I think that means that there is something platform-specific in the basic event handling on Mac.

I first suspected my recent Dock menu addition, but reverting that didn't help. What does help is removing the QSignalBlocker in Container::setCurrentWidget(). I haven't noticed any other effect of that modification; is it still required elsewhere (IOW, can we just remove it unconditionally)?

diff --git a/sublime/container.cpp b/sublime/container.cpp
index 43e280ff2d1a75efa60f72a419628ab3e2805fe4..972f36953f3112a70ac3643b24bc89a85090baa3 100644
--- a/sublime/container.cpp
+++ b/sublime/container.cpp
@@ -495,7 +500,9 @@ void Container::setCurrentWidget(QWidget* w)
     //this function is called from MainWindow::activateView()
     //which does the activation without any additional signals
     {
+#ifndef Q_OS_MACOS
         QSignalBlocker blocker(d->tabBar);
+#endif
         d->tabBar->setCurrentIndex(d->stack->indexOf(w));
     }
     if (View* view = viewForWidget(w))
Comment 1 RJVB 2017-08-06 14:51:19 UTC
ping?
Comment 2 Kevin Funk 2017-08-07 08:10:17 UTC
>  I haven't noticed any other effect of that modification; is it still required elsewhere (IOW, can we just remove it unconditionally)?

The signal-blocking code was added here:
  https://commits.kde.org/kdevplatform/e626a9715b3c79dc06ce413cac57203c13868823

Can you check the bug it tries to fix is still gone after removing the QSignalBlocker unconditionally?

Note: This discussion should happen on Phabricator.
Comment 3 RJVB 2017-08-07 09:13:46 UTC
OK: https://phabricator.kde.org/D7179
Comment 4 RJVB 2017-08-07 14:46:58 UTC
Git commit 4972dd5e10b070f815822943d9b9efe719ebbcd3 by René J.V. Bertin.
Committed on 07/08/2017 at 14:46.
Pushed by rjvbb into branch 'master'.

fix sortedDocumentList menu not changing focus on Mac

Sublime::Container::setCurrentWidget() used a QSignalBlocker to prevent
MainWindow::activateView() from being called twice. As a result, the
focus wasn't updated when changing documents with the sortedDocumentList
menu widget on Mac. This commit fixes that side-effect by using a simpler
approach where both functions only take action when setting/activating a
widget/view that is not already current.
Differential Revision: https://phabricator.kde.org/D7179

M  +5    -7    sublime/container.cpp
M  +1    -1    sublime/mainwindow.cpp

https://commits.kde.org/kdevplatform/4972dd5e10b070f815822943d9b9efe719ebbcd3