Fix lister unintended selection while scrolling
ClosedPublic

Authored by martinkostolny on May 7 2017, 12:47 AM.

Details

Summary

When Lister is scrolled (e.g. by mouse wheel) - first down and then back up -> some of the text is usually selected (visually) in the process. This patch should fix that.

This regression was introduced by me in the past when cursor blinking was added to Lister.

Test Plan

Up and down scrolling through a document + clicking on various places in the text and then scrolling up and down again.

Diff Detail

Repository
R167 Krusader
Lint
Lint Skipped
Unit
Unit Tests Skipped
martinkostolny created this revision.May 7 2017, 12:47 AM
abika added a subscriber: abika.May 12 2017, 5:32 PM

I usually never user krViewer.

Can confirm but its not totally fixed:

  1. Select one line
  2. Scroll down
  3. Scroll up

-> everything is selected between previously selected line and upper scroll position.

Thanks for testing it out, Alex. Finally I've got to this. In order to find and address the scrolling issue - some refactoring has been done:

  • small code simplifications and deduplications throughout the lister.cpp (there is still room for further deduplications)
  • replaced new & delete char* cache in favour of safe QByteArray
  • removed constant updating of file state and position (updates only when search is in progress)
  • added tun of const
  • increased cache size -> which fixed issue with searching up the document (previously it found only some of supposed matches)
  • fixed scrolling issue
  • simplified code for remote file handling

Hopefully I didn't break anything :).

asensi added a subscriber: asensi.Aug 29 2017, 10:51 AM

Hi!

With the latest improvements, at least using Kubuntu 17.04, not everything was perfect :-) :

  • if the user utilizes Lister to see a file like the "bashrc" attached one and then presses PgDown a lot of times: the scroll doesn't stop at the end of the file.
  • if the user utilizes Lister to see a file like the "profile" attached one and then presses PgDown: the cursor does not move.
  • if the user utilizes Lister to see a file like the "profile" attached one and then presses Ctrl+End: the cursor does not move.

Thanks!


Thanks for testing, Toni! This is what I needed. Here is a fix for your findings :).

I've found one other issue myself today - zooming out would cause that there are too many characters on one line not fitting the viewport width. This should now be fixed, too.

asensi added a comment.EditedAug 30 2017, 8:24 AM

I've found one other issue myself today - zooming out would cause that there are too many characters on one line
not fitting the viewport width. This should now be fixed, too.

I've done some more tryings, and if the user has some text selected, and he goes zooming in and out enough times: in the end the user sees that there are more selected characters than he previously chose, and sometimes fewer selected characters (this issue is not too important, maybe it has to do with what you wrote: "quite accurate (although not perfect) way [...]").

If the user enters the attached "try.zip" file, presses F3 over the "[Content_Types].xml" file, presses Ctrl+Shift+L to use Lister, then the user sees nothing, but if he presses (for example) PgDown then the (not syntax-highlighted) content appears (this seems to be easier to solve).

If the user presses F3 over the attached "chars.txt" file (with non us-ascii characters), then he sees "àèìòù", but if he presses Ctrl+Shift+L afterwards, then he sees ten hexagons with a "?" inside (this seems to be easier to solve).

And the previous found bugs were solved, very good! (it wasn't easy :-) )


Toni, as always, thanks a lot for your testing!

  • ad selected characters) I was trying hard but was unable to replicate that. I'm probably missing a specific pattern. I've tried selecting some characters within one line as well as through several lines, zooming in and out but was out of "luck". But so far I'm quite sure that the comment "quite accurate (although not perfect) way [...]" is not related to this issue :).
  • ad try.zip) fixed
  • ad chars.txt) fixed

Next fixes:

  • Ctrl+End should now go to the _end_ of last line in document
  • initial cursor position when document is opened is now 0, not -1, which was causing problems with text selection
  • opening a remote file is now properly updating view and file size when the file is being downloaded
asensi accepted this revision.EditedSep 1 2017, 9:06 PM

For my part, I accept this because every of my performed tests has been successful, even the "selected characters" test :-). Other people can do their checks.

This revision is now accepted and ready to land.Sep 1 2017, 9:06 PM
This revision was automatically updated to reflect the committed changes.

Thanks a lot for your thorough testing :).