Resize content area to correct dimensions after zooming to Auto Fit
Needs RevisionPublic

Authored by chinmoyr on Apr 10 2018, 7:15 PM.

Details

Reviewers
aacid
ngraham
Group Reviewers
Okular
Summary

After going from high zoom to "Auto Fit" a resize event is generated and the resize event handler
tries to refresh the page view. Sometimes this can result in an infinite resize loop. Okular
fixes this by resizing the content area to the new viewport size. As a consequence both horizontal
and vertical scroll range become zero. Now if "Auto Fit" turns out to be "Fit Page" then it is fine
but in case of "Fit Width" or "Fit Height", where one scroll range should be non-zero, both scroll
bar disappear resulting in a view which appears to be frozen. So to fix this add scroll range
to the dimension for which the scroll bar should be visible and then resize. Since the scroll range
remains constant, an infinite resize loop won't occur.

BUG: 341250

Test Plan

Zoom in 400% and then select "Auto Fit".

Diff Detail

Repository
R223 Okular
Branch
master
Lint
No Linters Available
Unit
No Unit Test Coverage
chinmoyr created this revision.Apr 10 2018, 7:15 PM
Restricted Application added a project: Okular. · View Herald TranscriptApr 10 2018, 7:15 PM
chinmoyr requested review of this revision.Apr 10 2018, 7:15 PM
ngraham accepted this revision.Sep 20 2018, 8:45 PM
ngraham added a subscriber: ngraham.

Works for me: fixes the bug and doesn't seem to regress things after a number of other move and zoom operations.

Okular folks, what do you think? It would be great if we could review some of these patches.

This revision is now accepted and ready to land.Sep 20 2018, 8:45 PM
Restricted Application added a subscriber: okular-devel. · View Herald TranscriptSep 20 2018, 8:45 PM
sander added a subscriber: sander.Oct 1 2018, 9:28 AM

I can indeed get the UI to freeze by zooming in a lot and then enabling Auto Fit. The patch fixes this for me, but to be honest I don't understand the code well enough whether the diff fixes the problem in the most appropriate way.

@aacid, does this make sense to you?

aacid added a comment.Oct 1 2018, 9:52 PM

This doesn't make sense to me, this code should not be triggered in the case that is higlighted in this bug since AutoFit in that very long document should never be a case of "the scrollbar vs no scrollbar fight", so what we need to figure out is why the if is being triggered and protect against it, not fixing what is inside the if.

aacid requested changes to this revision.Oct 1 2018, 9:52 PM
This revision now requires changes to proceed.Oct 1 2018, 9:52 PM