Bug 563531 - [regression][StyledText] Scrolling with arrow down key does not update caret painting
Summary: [regression][StyledText] Scrolling with arrow down key does not update caret ...
Status: VERIFIED FIXED
Alias: None
Product: Platform
Classification: Eclipse Project
Component: SWT (show other bugs)
Version: 4.16   Edit
Hardware: All All
: P3 major (vote)
Target Milestone: 4.16 RC1   Edit
Assignee: Niraj Modi CLA
QA Contact:
URL:
Whiteboard:
Keywords: regression
Depends on: 168557
Blocks:
  Show dependency tree
 
Reported: 2020-05-24 18:16 EDT by Paul Pazderski CLA
Modified: 2020-05-26 10:49 EDT (History)
6 users (show)

See Also:
lshanmug: review+


Attachments
Wrong painted caret in source editor (310.55 KB, image/gif)
2020-05-24 18:16 EDT, Paul Pazderski CLA
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Paul Pazderski CLA 2020-05-24 18:16:32 EDT
Created attachment 283004 [details]
Wrong painted caret in source editor

Move the caret using the arrow up/down keys. Once you reach the viewport bottom and the StyledText need to be scrolled the visible(!) caret stands still, i.e. does not move until pressing another key. Same for the opposite direction.

See the attached video to understand the issue. It shows a source editor but the issue also exist in a plain StyledText, e.g. Snippet376. Happens with and without word wrap.


The regression comes from bug 168557.
https://git.eclipse.org/r/#/c/159376/

It tries to calculate the new caret location without using getPointAtOffset. That works for some types of scrolling but not from scrolling by LINE_UP or LINE_DOWN.
Another variant where the caret is not correctly updates is if the last visible line is only partial shown and you do a mouse click in this partial visible line. The content is scrolled to make the line fully visible, the next input happens where you clicked, but the caret is painted in its old position.
Comment 1 Eclipse Genie CLA 2020-05-24 18:38:41 EDT
New Gerrit change created: https://git.eclipse.org/r/163500
Comment 2 Paul Pazderski CLA 2020-05-24 18:39:37 EDT
Added a small test case for this regression. The test succeeds (at least on Windows) before bug 168557.
Comment 3 Andrey Loskutov CLA 2020-05-25 05:15:32 EDT
Sounds like quite severe regression. Revert bug 168557 fix?
Comment 4 Lakshmi P Shanmugam CLA 2020-05-26 01:51:56 EDT
I wasn't able to reproduce the problem with Eclipse on Mac. But the test case in gerrit fails on Mac too and it passes on reverting bug 168557.

Mickael/Conrad, can you please take a look?
Comment 5 Niraj Modi CLA 2020-05-26 02:16:46 EDT
(In reply to Andrey Loskutov from comment #3)
> Sounds like quite severe regression. Revert bug 168557 fix?

Partially reverting one of the change w.r.t. to "setCaretLocation()" from bug 168557 fix, resolves this issue. Sharing a gerrit shortly.
Comment 6 Eclipse Genie CLA 2020-05-26 02:20:06 EDT
New Gerrit change created: https://git.eclipse.org/r/163570
Comment 7 Niraj Modi CLA 2020-05-26 03:59:50 EDT
Hi Lakshmi,
For your review+ for RC1
Comment 10 Niraj Modi CLA 2020-05-26 10:19:53 EDT
Thanks Andrey for the review and Paul for the JUnit test, resolving this bug now.
Comment 11 Andrey Loskutov CLA 2020-05-26 10:49:21 EDT
Verified with build I20200526-0600