Bug 402760 - Source browse mode activated when saving the document with shortcut
Summary: Source browse mode activated when saving the document with shortcut
Status: RESOLVED FIXED
Alias: None
Product: kdev-python
Classification: Developer tools
Component: general (show other bugs)
Version: 5.3.2
Platform: Other Linux
: NOR major
Target Milestone: ---
Assignee: Sven Brauch
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2019-01-01 17:09 UTC by Simone Gaiarin
Modified: 2019-10-13 06:28 UTC (History)
2 users (show)

See Also:
Latest Commit:
Version Fixed In:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Simone Gaiarin 2019-01-01 17:09:31 UTC
SUMMARY
In KDevelop 5.3.1 when I place the mouse over a variable/function the mouse pointer becomes a "click hand" and clicking on the variable moves the text cursor to the definition of the variable even if "Source browse mode" is disabled.

STEPS TO REPRODUCE
1. With the editor selected, save the document with Ctrl+S > 

OBSERVED RESULT
Cursor becomes a hand when hovering variables (Source browse mode does not result enabled though)

EXPECTED RESULT
Nothing should happen

SOFTWARE/OS VERSIONS
Operating System: Manjaro Linux 
KDE Plasma Version: 5.14.4
Qt Version: 5.12.0
KDE Frameworks Version: 5.53.0
Kernel Version: 4.19.13-1-MANJARO
OS Type: 64-bit


ADDITIONAL INFORMATION
2. Click outside the editor > everything goes back to normal
3. Select the editor and Ctrl+S > cursor becomes a hand when hovering variables

See:https://forum.kde.org/viewtopic.php?f=218&t=156466
Comment 1 Simone Gaiarin 2019-01-01 17:23:41 UTC
Probably related: after saving it is not possible to type numbers. Again clicking outside the editor and clicking back to it, it is possible to type numbers again.
Comment 2 RJVB 2019-01-01 19:36:08 UTC
I can confirm both observations (KF5 5.52.0 and Qt 5.9.7). The symptom with typing numbers is new AFAICT, I only noticed it for the first time today.

This reminds me of a long-standing issue on Mac, where sometimes all input becomes "suspended" and I have to switch key focus to and from a different application. I don't think that's related to saving, but I do know I've already noticed it after switching to the patchreview view.

I think this is a major issue because it happens so frequently (and because of the keyboard input aspect if it's indeed related).
Comment 3 Simone Gaiarin 2019-05-04 07:08:23 UTC
The behavior is triggered when the editor is selected and Ctrl is pressed. So saving the document is not related indeed.
Comment 4 RJVB 2019-05-04 08:18:25 UTC
"Saving with shortcut" is (unless maybe if you manage to hit Ctrl and S at exactly the same time).

Seems odd to use the "shortcut moderator" key in itself, no?
Comment 5 Simone Gaiarin 2019-05-04 18:13:31 UTC
I haven't understood your point. I have not set Ctrl as a shortcut for save. I am using the default shortcuts.

I compiled KDevelop from source (tag 5.3.2), and when executed it does not have this problem. Only the version installed through the package manager has it.

Any idea how can I further debug this?
Comment 6 RJVB 2019-05-04 22:26:44 UTC
I didn't say you changed the "save" shortcut, I pointed out that it is difficult to use that or any other shortcut without side-effect if the Ctrl key on it self is used as a shortcut too.
Comment 7 Simone Gaiarin 2019-05-05 06:37:57 UTC
Ok. Now I got it. 

Can you share your software/os versions? Maybe it is not related to KDevelop, given that the version from compiled from source works. Maybe it is some other KDE part.
Comment 8 RJVB 2019-05-05 07:28:12 UTC
kdevelop v5.3.2-7-g566b98a996, frameworks 5.52.0, Qt 5.9.8 (all built from source) on both Mac and Linux.

Can you point me to the source location where Ctrl is set as the shortcut to activate the source browse mode?
Comment 9 Simone Gaiarin 2019-05-05 08:27:46 UTC
It is not in the source. My comment was about what I observe using KDevelop. Once I select the editor and click Ctrl the pointer becomes a hand (as in source browse mode, even though this is not actually activated Navigation > Source Browse mode is unchecked). It was just an edit on what I stated before, i.e. that it was Ctrl+S that triggers the problem, instead it is just Ctrl.
Comment 10 RJVB 2019-05-05 09:03:05 UTC
Well, it has to be in the code somewhere, otherwise it would really be a bug (a mutant one even) ;)
Comment 11 Simone Gaiarin 2019-05-09 08:02:37 UTC
The bug is in kdev-python. Uninstalling that package the problem goes away (reason why I could not see the problem in the compiled version of KDevelop, because it was not loading kdev-python correctly).
Comment 12 RJVB 2019-05-09 08:41:29 UTC
Interesting. Did you check if just disabling the plugin from the global settings dialog is enough to make the issue go away (immediately or after restarting KDevelop)?
Comment 13 Simone Gaiarin 2019-05-09 08:58:23 UTC
Disabling the plugin "python support" and restarting KDevelop solves the problem.
Comment 14 RJVB 2019-05-09 09:48:52 UTC
That's great to know because plugin enabled/disabled state info is per-session, meaning you can keep the python plugin disabled except in sessions dedicated to python development.

(Sadly it's a bit hit-and-miss to predict what plugins will be active when you create a new session in my experience, but that's a different issue.)
Comment 15 Simone Gaiarin 2019-05-11 19:08:41 UTC
Some thoughts from my debug session:

The culprit is in browsemanager.cpp, method eventFilter (Kdevelop at tag 5.3.2)

When the Ctrl key is pressed, we enter the first 'if' (line 178) because keyEvent->key() == browseKey
and browseMode is activated. m_browsingByKey is set.
The event at this point is QKeyEvent(KeyPress, Key_Control, ControlModifier)

When Ctrl is released, m_broswingByKey should be unset at line 216, but here we fail to enter the if because we have QKeyEvent(KeyRelease, 0, ControlModifier), so that the condition keyEvent->key() == m_browsingByKey is not satisfied.

Given that m_broswingByKey is not unset, as soon we move the mouse we enter eventFilter again and we skip the if at line 233, where we should reset the cursor and return. Instead we proceed to the next if where setHandCursor is called.

Basically the problem is that the event we receive when we release Ctrl is QKeyEvent(KeyRelease, 0, ControlModifier) instead of QKeyEvent(KeyPress, Key_Control, ControlModifier) and we miss it.


For some reason when the python language plugin is not active we re-enter eventFilter few more times after moving the mouse up to the point where m_browsingByKey is unset and everything goes back to normal. (This interaction is still not clear to me). Nonetheless I think the problem is not in the plugin but in KDevelop itself. It should catch the KeyRelease event correctly.

I see if I can think a way to fix this.
Comment 16 RJVB 2019-05-15 09:00:04 UTC
That doesn't look like it's a shortcut that could be put in the list of configurable shortcuts?

Frankly, who's idea was it to use the Ctrl key for this feature? It seems evidently inavoidable that this would lead to unwanted side-effects with the use of that key as the "shortcut opcode"... Is the feature even documented somewhere?
 IIRC there is a standard shortcut for toggling source browse mode that *can* be configured. If so and assuming there are people who don't want the feature to change (use a different key, for instance) it easiest workaround would be to add a setting to disable the feature if no reliable fix can be found.

Are mouse events the only possible culprits that can derail the algorithm? I never observed this link but then again I presume we don't really think about using a shortcut to save the document or copy a snippet and moving the mouse cursor with the other hand.
Comment 17 Simone Gaiarin 2019-05-15 11:13:27 UTC
Alt is already used to show the tooltip as documented here:

https://userbase.kde.org/KDevelop5/Manual/Working_with_source_code#Exploring_source_code

Moreover, both Ctrl and Alt achieve the same effect of letting the user click on a variable/method name and navigate to it. This feature seems not documented.

I suggest to:
- Drop Ctrl and keep Alt to activate the mode that let us navigate the code
- Document this feature
Comment 18 Simone Gaiarin 2019-05-15 11:20:12 UTC
To fully reply to you. There is indeed a standard shortcut to activate source browse mode. Though, I think that having a quick modifier to enable it for a second while clicking is not bad (this bug let me discover this feature and I started using it with pleasure). Switching completely to source browse mode while I am coding and not analyzing code seems too heavy, I would keep switching back and forth continuously.

I think that this feature needs to be implemented using a modifier, and this cannot be configured through the standard shortcut configuration window.

Using Alt, that is already used to show the tooltip seems not to cause any evident side-effect.
Comment 19 RJVB 2019-05-15 11:41:22 UTC
I hesitated suggesting to use Alt exactly because it already has the tooltip function, and on Linux and MSWin it also serves as a menu activator.
Comment 20 Simone Gaiarin 2019-05-15 13:45:54 UTC
Regarding Alt having already the function of the tooltip, that is true, but on the other hand that function is used only when the user interact with the keyboard, so that the user does not even notice that Alt is enabling source browse mode on the code (because the mouse is not moved). When instead the user uses the mouse, Alt is irrelevant to show the tooltip, because the tooltip appears on mouse hoover and Alt can be used to enable source browse mode.

So I think all in all keeping Alt for source browse mode it is an ok solution for now, and it is also backward compatible given that it was already there.