When changing the mouse tool to text selection, okular sometimes behaves as though the mouse button is already pressed, forcing the user to click once to cancel and again to begin text selection. Reproducible: Always Steps to Reproduce: 1. Create an annotation if one does not already exist. 2. Select the browse tool 3. Right click on the annotation 4. Hit escape to cancel the pop-up menu. 5. Change to the text selection tool Actual Results: Text selection begins immediately, as though a mouse button was held down. Expected Results: Text selection should not begin until a mouse button is pressed at the start of the desired selection. The PageView class loses track of its internal state. In part this is related to the fact that the annotation pop-up menu appears when the right mouse button is pressed, rather than when it is released, as all other pop-up menus do.
I cannot confirm the "text selection begins immediately" part, neither with the okular from git master nor the one from Debian testing. What I can confirm is that the annotation context menues really do appear on mouse-press rather than on mouse release.
Created attachment 104034 [details] PDF that demonstrates bug
Created attachment 104035 [details] Screen capture video demonstrating bug
I just reproduced the problem once again using Okular from Debian/unstable. I've attached a copy of the PDF and a screen capture video to demonstrate. The important bit to notice is at 0:22 after I choose "Text Selection Tool" from the "Tools" menu. I then move the mouse across the text, which begins selecting even though I have not pressed the mouse button to begin selection.
Created attachment 104037 [details] Patch to fix problem It's not very elegant, but this patch solves the problem. It does this by creating a function handleGenericRightButtonRelease, partly replacing handling right-click in different places in the code. It has the beneficial side-effect of resolving the inconsistent response to right-clicking while in text selection mode as described here: https://git.reviewboard.kde.org/r/127496/
Now I can confirm the problem, both with the Debian version of okular and the one from git master. My problem was > 5. Change to the text selection tool I was using the keyboard short-cut for this, and then there is no bug. When I use the menu then text selection indeed begins immediately.
Created attachment 104443 [details] Alberts patch to fix the problem This is a much simpler fix to the problem, i'll commit it in ~24 h if noone disagrees. About the other review you posted, shit it's been sitting there for almost a year. Someone should really have a look at it.
Comment on attachment 104443 [details] Alberts patch to fix the problem Sure that is a simple and effective solution to this bug. However it does nothing for two identified inconsistencies: 1. annotation context menus appearing on mouse-press instead of mouse-release; and 2. differing responses in different modes for no good reason to right-click.
Git commit d3b5e13dade487184d07d2c0c42848b7ae0268f3 by Albert Astals Cid. Committed on 08/03/2017 at 23:38. Pushed by aacid into branch 'master'. Reset d->mousePressPos correctly in mousePress after popup exec M +3 -0 ui/pageview.cpp https://commits.kde.org/okular/d3b5e13dade487184d07d2c0c42848b7ae0268f3
(In reply to Jonathan from comment #8) > Comment on attachment 104443 [details] > Alberts patch to fix the problem > > Sure that is a simple and effective solution to this bug. However it does > nothing for two identified inconsistencies: 1. annotation context menus > appearing on mouse-press instead of mouse-release; and 2. differing > responses in different modes for no good reason to right-click. Those are possibly valid issues but unrelated to the "Text selection begins prematurely followint change to tool" bug.
I beg to differ. The failure to set d->mousePressPos is a direct consequence of handling this particular click differently from every other one. So although the patch removes one obvious malfunction, it is one more step down the road of inconsistent coding and function in pageview.