Bug 421822

Summary: 'Scroll Page Up'/'Scroll Page Down' actions scroll by scrollbar height instead of viewport height, which is incorrect when scrollbars are disabled
Product: [Applications] okular Reporter: quantera
Component: generalAssignee: Okular developers <okular-devel>
Status: RESOLVED FIXED    
Severity: major CC: aacid, durakov96, eric.brunet, kdudka, keziolio123, nate, oliver.sander, quantera, triffid.hunter, yurchor, yuricampelo.ufrn
Priority: NOR Keywords: regression
Version: 20.12.0   
Target Milestone: ---   
Platform: unspecified   
OS: All   
Latest Commit: Version Fixed In: 21.08
Attachments: Demonstration of a problem
Prevent pgup/pgdn from triggering problematic "smooth" scroll
Pressing and holding PgDn in version 19.12.3
Pressing and holding PgDn in version 20.08.1

Description quantera 2020-05-20 13:28:50 UTC
Created attachment 128636 [details]
Demonstration of a problem

SUMMARY

After upgrade to okular 20.0.4, keyboard shortcuts bound to 'Scroll Page Up'/'Scroll Page Down' actions do not scroll by screenfulls, like they should, but instead behave similarly to 'Arrow Up'/'Arrow Down' keys.
Those shortcuts work properly in versions 19.12.3 or below

STEPS TO REPRODUCE

Open any pdf document and try to scroll down by using either PgDown or Space keys (which are bound to 'Scroll Page Down' by default)

OBSERVED RESULT

The document is scrolled by small increments

EXPECTED RESULT

The document should scroll by screenfulls

SOFTWARE/OS VERSIONS

Linux: Linux arch 5.6.11-arch1-1 x86_64 GNU/Linux
KDE Plasma Version: 5.18.5
KDE Frameworks Version: 5.69.0
Qt Version: 5.14.2

ADDITIONAL INFORMATION

Attached is a screencast which demonstrates the problem. In this video, after a short initial pause I press PgDown (or Space) button 10 times, then after some pause I press PgDown and leave it pressed for a few seconds.
Comment 1 quantera 2020-05-20 14:36:42 UTC
I would like to make a correction to my initial report: the problematic version is not 20.04, but 1.10.1 (and 1.10.0 as well), while a properly working version is 1.9.3 and below. I just realized that versions 19.12.3 and 20.04 (and not 20.0.4, as I written originally) are package versions in Archlinux, and not okular versions.
Comment 2 Albert Astals Cid 2020-05-20 22:01:29 UTC
Can't reproduce myself
Comment 3 quantera 2020-05-21 15:19:28 UTC
It is a bit strange that the problem doesn't reproduce in your environment. Initially I asked about the problem in arch's forum (here: https://bbs.archlinux.org/viewtopic.php?id=255604), and waited for somebody to confirm the problem before submitting the bug report - to make sure that the problem is not specific to my machine. 

I see that arch's package just builds okular as is, without adding any distribution-specific patches (here is the relevant link: https://git.archlinux.org/svntogit/packages.git/tree/trunk/PKGBUILD?h=packages/okular), therefore I would expect the problem to be easily reproducible. 

In addition, the problem does not seem to be caused by okular's dependencies, since when I downgrade okular to a previous version (without changing anything else on the machine), the problem disappears.

I tried to bisect the problem in order to find the exact commit which introduced the problem, but unfortunately I am unable to execute the manually built okular: when I try to run it, I get an error message saying: "Unable to find Okular component: The shared library was not found", which is somewhat strange, since I build the program with the following arguments:

cmake -DCMAKE_INSTALL_PREFIX=<path to some dir in my homedir> -DCMAKE_BUILD_TYPE=Debug <path to checked out okular's source

and then 'make install'

Moreover, I see that the built exe is built with a correct RUNPATH (by running 'objdump -x' on it), and running ldd on okular I don't see any missing libs.

Any suggestions about how I can proceed with finding the problem? If I just have been able to compile the okular myself, I would have been able to provide a much better input after bisection
Comment 4 Yuri Chornoivan 2020-05-21 15:22:53 UTC
(In reply to quantera from comment #3)
> cmake -DCMAKE_INSTALL_PREFIX=<path to some dir in my homedir>
> -DCMAKE_BUILD_TYPE=Debug <path to checked out okular's source
> 
> and then 'make install'

-DKDE_INSTALL_PLUGINDIR=/usr/lib64/qt5/plugins

should be added to make it work.
Comment 5 quantera 2020-05-21 20:04:20 UTC
(In reply to Yuri Chornoivan from comment #4)

> -DKDE_INSTALL_PLUGINDIR=/usr/lib64/qt5/plugins
> should be added to make it work.

Thanks for the hint - eventually I found out that what I needed was QT_PLUGIN_PATH, which should be set to <path to some dir in my homedir>/lib/plugins, and then indeed okular succeeds to run. The KDE_INSTALL_PLUGINDIR is used only as an installation path, and since I am not running as a root I cannot install compilation results into a system-wide dir, especially since I want to run multiple versions.

In any case, now I have the two points for bisection (v19.12.3 & v20.04.1), where the problem occurs in one but not in another. I will report back when I find the commit that introduces the problem.
Comment 6 quantera 2020-05-22 10:04:37 UTC
The commit that introduced the problem is 51b90ecd73e37b9646d8a4bbb51e4fa815942912. It's parent commit 13bb95c253332145db0609ffd8fbdad3bed49fff is fine.

I tried to revert the commit from a top of the master branch to see if it is the only commit that is related to the problem, but there were further changes in the same code in later commits and therefore the revert caused merge conflicts. I am not familiar with the code to resolve those conflicts properly, so I didn't proceed with this.
Comment 7 Boris 2020-06-07 07:40:59 UTC
I have the same issue on Arch Linux. I have to downgrade to version 19.12.3.
Comment 8 yuricampelo.ufrn 2020-06-20 11:22:28 UTC
I am using it on windows 10 and facing the same problem.
Comment 9 triffid.hunter 2020-08-13 04:38:04 UTC
This is infuriating me as well, I'm used to being able to fly through documents with pgup/pgdn but this has become impossible
Comment 10 triffid.hunter 2020-08-13 06:22:51 UTC
Created attachment 130840 [details]
Prevent pgup/pgdn from triggering problematic "smooth" scroll

Here's a quick kludge to hopefully invoke Cunningham's law.

If you have pageup/pagedn assigned in the keyboard shortcuts, you may need to unassign them for this to have any effect.
Comment 11 Kamil Dudka 2020-09-10 13:30:08 UTC
(In reply to triffid.hunter from comment #10)
> Created attachment 130840 [details]
> Prevent pgup/pgdn from triggering problematic "smooth" scroll

This patch fixed the issue for me.  I have a usable okular on my system again.  Thanks!
Comment 12 triffid.hunter 2020-09-10 16:02:50 UTC
I've found that my patch works if you stick with pgup/pgdn, but the scroll position gets confused and jumps suddenly if you subsequently use scroll wheel - apparently I need to update another control somewhere for it to work properly.

What exactly I'm missing I'm not sure - my bread and butter is firmware, not Qt/GUI programming ;)

I'm keeping the patch in my local set, this new glitch is vastly less irritating than the issue this bug is about, but I hope one of y'all can make a better patch and post it here!
Comment 13 Kamil Dudka 2020-09-11 06:16:49 UTC
It might be inperfect but at least it is usable again.  I am not a GUI developer either, so I will rather not touch the code myself.  I would prefer if this bug was fixed by fully reverting the change that introduced the fancy animation on pgup/pgdn.
Comment 14 triffid.hunter 2020-09-11 08:28:44 UTC
I tried reverting the commit that quantera found, but there's been too much stuff built on top of it since then and I ended up with a huge mess.

I also tried to backtrack, revert, replay but that horribly exploded too.
Comment 15 Nate Graham 2020-09-14 17:14:18 UTC
I'm not able to reproduce the bug. PgUp/PgDn and Space/Shift+Space scroll by whole screenfuls, not as arrow keys.

Are we all talking about the same thing?
Comment 16 triffid.hunter 2020-09-15 04:41:23 UTC
(In reply to Nate Graham from comment #15)
> I'm not able to reproduce the bug. PgUp/PgDn and Space/Shift+Space scroll by
> whole screenfuls, not as arrow keys.
> 
> Are we all talking about the same thing?

For me the issue is mostly the smooth scroll animation that occurs on pgup/pgdn rather than the specific scroll distance - but both are annoying when I'm used to being able to page down 30 pages in a second or two by just holding pgdn, and both seem to be caused by the same commit indicated in comment 6
Comment 17 Kamil Dudka 2020-09-15 13:38:41 UTC
(In reply to Nate Graham from comment #15)
> Are we all talking about the same thing?

Nope.  For me the issue is the smooth scroll animation that blocks the scrolling.  It makes it impossible to scroll large documents quickly with keyboard.

I believe that the scroll distance can be to some extent configured as mentioned in another bug: Settings -> Configure Okular -> General -> Page Up/Down overlap
Comment 18 Nate Graham 2020-09-15 15:53:53 UTC
Got it. Looks like everyone's actually complaining about Bug 422050, which was just fixed! :)

*** This bug has been marked as a duplicate of bug 422050 ***
Comment 19 quantera 2020-09-15 16:15:49 UTC
Created attachment 131679 [details]
Pressing and holding PgDn in version 19.12.3
Comment 20 quantera 2020-09-15 16:16:32 UTC
Created attachment 131680 [details]
Pressing and holding PgDn in version 20.08.1
Comment 21 quantera 2020-09-15 16:19:55 UTC
The problem is still present in version 20.08.1 (or 1.11.1, as is shown in 'About Okular' dialog). I attached two videos - in both of them I open a pdf file, and press & hold PgDn button. One video is for version 19.12.3 (1.9.3) - here scrolling works as expected. The second one is for version 20.08.1 (1.11.1), where things don't work properly, as can be seen.
Comment 22 Nate Graham 2020-09-15 16:30:20 UTC
20.08.1 doesn't have the fix. It'll be in 20.08.2. See the duped bug for details.

*** This bug has been marked as a duplicate of bug 422050 ***
Comment 23 Kamil Dudka 2020-09-15 16:44:45 UTC
(In reply to Nate Graham from comment #18)
> Got it. Looks like everyone's actually complaining about Bug 422050, which
> was just fixed! :)

Yes, I was facing exactly this bug.  Thank you for pointing me to the fix!
Comment 24 Éric Brunet 2021-02-11 23:10:31 UTC
I think there's been a mix up between two different issues

a) the fact that Scroll Up/Scroll Down or Space moved the document by about one line of text instead of nearly the full displayed height

b) the issues about smooth scrolling

The original description of the bug was about problem a), but in the comments it drifted to problem b), and eventually the bug was marked as resolved and duplicate of bug 422050. But this is for problem b)! 

Problem a) is not fixed, and I am reopening this bug.

My okular was working correctly in Fedora 32 (Version 19.something of okular, I didn't make note of exact version). I upgraded to Fedora 33 (Version 20.08) and ran into the problem described here. I then upgraded the okular package from fedora rawhide (or 34) in the hope the problem was already fixed:

okular-20.12.2-2.fc34.x86_64

But it is not.

The bug seem related to scrollbars. I have set up okular so that scrollbars are not set. Then:
1) I run okular on some pdf file. No scrollbar, of course, and the bug is present: pressing the space key moves me down by about one line of text.
2) I go into the settings and I ask for the scrollbars to be displayed
3) The bug no longer shows! Pressing Space scrolls me down by nearly one height of the displayed text.
4) I go back into the settings
Comment 25 Éric Brunet 2021-02-11 23:16:23 UTC
[Sorry, I seem to have sent my comment prematurly. I am finishing it now]

4) I go back into the settings and ask for the scrollbars to be hidden
5) The space key is still working properly.
6) I quit okular and start again
7) The bug is is back: the space key scrolls by one line of text.

Extra information: I have a HiDPI display, and I might have some non-standard configuration options. For instance, my plasma desktop is scaled by a factor 2 and the server X is started with -dpi 192. I don't know whether this is relevant or not.

So the bug is still present in okular 20.12, but depends on some configuration options. In my case, to observe it, okular must be started without any scrollbar.
Comment 26 David Hurka 2021-02-12 10:58:45 UTC
I can confirm this on git master. Page Up/Down scroll only by approximately one scrollbar thickness when “Show scrollbars” is unchecked in the configuration, and scrollbars were not shown before in the current Okular session.

Apparently no one noticed before that the video demonstrations have scrollbars hidden.

The patch attached to this bug theoretically works fine, but disables smooth scrolling for Page Up/Down completely, so we need a different solution. Probably inside slotScrollUp/Down().
Comment 27 Nate Graham 2021-02-12 15:44:41 UTC
Can also confirm when scrollbars are disabled. Seems like that was a key piece of information missing from the original bug report. :)
Comment 28 David Hurka 2021-03-31 11:50:00 UTC
Page Up/Down probably ask for the height property of the vertical scrollbar. Until the scrollbar is shown for the first time, it has a default height, and Page Up/Down scroll by the wrong distance.

To check that, open a long document with scrollbars shown, then disable scrollbars, resize the window, and do Page Up/Down.

The fix for this bug is probably a +1-1 patch.
Comment 29 Nate Graham 2021-04-01 23:04:55 UTC
Yup, that's exactly what happens. Great intuition!
Comment 30 Bug Janitor Service 2021-04-04 13:22:56 UTC
A possibly relevant merge request was started @ https://invent.kde.org/graphics/okular/-/merge_requests/396
Comment 31 Nate Graham 2021-07-22 21:37:26 UTC
Git commit c399a11054fffafb86938077a0591c05a4a3511c by Nate Graham, on behalf of David Hurka.
Committed on 22/07/2021 at 21:05.
Pushed by ngraham into branch 'release/21.08'.

Fix Page Up/Down scrolling when scrollbars are turned off

PageView::slotScrollUp/Down() queried the size of the scrollbar instead of the viewport,
which has no obvious reason and doesn’t work when scrollbars are not there.

Probably the scrollbar size was thought to be more robust
in times when PageView had overshoot enabled.

Also fixes code documentation for these methods,
and removes another spaghetti feature where Continuous mode was queried
in an `if` branch before the one where it is relevant.

M  +10   -10   part/pageview.cpp

https://invent.kde.org/graphics/okular/commit/c399a11054fffafb86938077a0591c05a4a3511c