Bug 542107 - [StyledText] Vertical indent moves cursor without clicking the line
Summary: [StyledText] Vertical indent moves cursor without clicking the line
Status: CLOSED DUPLICATE of bug 570208
Alias: None
Product: Platform
Classification: Eclipse Project
Component: SWT (show other bugs)
Version: 4.10   Edit
Hardware: PC Linux
: P3 normal (vote)
Target Milestone: ---   Edit
Assignee: Platform-SWT-Inbox CLA
QA Contact:
URL:
Whiteboard: stalebug
Keywords: triaged
Depends on:
Blocks:
 
Reported: 2018-12-04 15:40 EST by Eric Williams CLA
Modified: 2021-02-10 03:02 EST (History)
4 users (show)

See Also:


Attachments
StyledText snippet with both vertical and horizontal indents (1.43 KB, text/x-java)
2018-12-04 15:40 EST, Eric Williams CLA
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Eric Williams CLA 2018-12-04 15:40:37 EST
Created attachment 276817 [details]
StyledText snippet with both vertical and horizontal indents

With addition of vertical indents in StyledText we have a new bug to consider.

Some background: with horizontal indents, clicking in the indented (blank) space will move the cursor to the start of the line. You can see this behaviour with the snippet attached: click anywhere in the blank space of the 2nd line's indent, and the cursor will jump to the start of that line.

With vertical indents, the same sort of behaviour is applied which IMO is incorrect. In the snippet provided: click anywhere in the blank space of the 1st line's vertical indent and you will see the cursor move to that position, just in the line below. If you click and drag in the vertical indent, the text beneath is selected. On mac the bug is somewhat present, it happens only when double clicking in the vertical indent

I think this is because of the way StyledText handles indents, i.e. that indents are horizontal and thus the mode of thinking lies with that behaviour in mind. However now that we have vertical indents, there are some other cases to consider which previously were not.

I haven't tested on Windows yet, if someone with a Windows machine could try the snippet attached that would be appreciated.
Comment 1 Angelo ZERR CLA 2018-12-04 17:34:20 EST
> I haven't tested on Windows yet, if someone with a Windows machine could try the snippet attached that would be appreciated.

I have tested in Windows OS and it seems that it follows your description issue, but to be honnest with you I don't see a problem with the cursor behaviour? I think I have missed something. Could you attach a demo (or several) which shows in action problems? Thanks!
Comment 2 Mickael Istria CLA 2018-12-05 03:09:01 EST
I don't think SWT is wrong here. If I'm an an editor (Writer or Eclipse IDE) and click the blank indent before a line, then I expect cursor to move to that line.
However, the annoying case is purely about code minings/annotations: it seems like when there is an annotation drawn, we'd like the MouseEvent to be consumed and not propagated to moving the cursor.
Comment 3 Eric Williams CLA 2018-12-05 10:30:06 EST
I think the issue stems more from the line of thinking around indents. Until now all indents were horizontal, so clicking in the blank space of an indent should automatically move the cursor. However I am not convinced that doing so in a vertical indent makes any sense.

Angelo, how is the code mining text being drawn? I assume the vertical indent is added to the line beneath it, and then there is some manual drawing of the text over where the indent is?
Comment 4 Angelo ZERR CLA 2018-12-05 10:34:24 EST
> Angelo, how is the code mining text being drawn? I assume the vertical indent is added to the line beneath it, and then there is some manual drawing of the text over where the indent is?

The LineHeaderCodeMining waits for a before line number which search the offset where line starts with indent (see call of Positions.of(beforeLineNumber, document, true))
Comment 5 Mickael Istria CLA 2018-12-05 10:45:34 EST
(In reply to Eric Williams from comment #3)
> I think the issue stems more from the line of thinking around indents. Until
> now all indents were horizontal, so clicking in the blank space of an indent
> should automatically move the cursor. However I am not convinced that doing
> so in a vertical indent makes any sense.

Your POV is arguable. I have the opposite one ;)
We need more opinions here to take a decision and stick to it.

> Angelo, how is the code mining text being drawn? I assume the vertical
> indent is added to the line beneath it, and then there is some manual
> drawing of the text over where the indent is?

The code mining sets the line indent to create the space and draws manually in it.
Comment 6 Eric Williams CLA 2018-12-05 11:23:52 EST
(In reply to Mickael Istria from comment #5)
> (In reply to Eric Williams from comment #3)
> > I think the issue stems more from the line of thinking around indents. Until
> > now all indents were horizontal, so clicking in the blank space of an indent
> > should automatically move the cursor. However I am not convinced that doing
> > so in a vertical indent makes any sense.
> 
> Your POV is arguable. I have the opposite one ;)
> We need more opinions here to take a decision and stick to it.

Fine by me. :) Alex, Lakshmi -- what do you think?

> > Angelo, how is the code mining text being drawn? I assume the vertical
> > indent is added to the line beneath it, and then there is some manual
> > drawing of the text over where the indent is?
> 
> The code mining sets the line indent to create the space and draws manually
> in it.

Okay this is what I thought -- just confirming.
Comment 7 Lakshmi P Shanmugam CLA 2018-12-06 08:06:47 EST
(In reply to Eric Williams from comment #0)
> 
> With vertical indents, the same sort of behaviour is applied which IMO is
> incorrect. In the snippet provided: click anywhere in the blank space of the
> 1st line's vertical indent and you will see the cursor move to that
> position, just in the line below.

Hi Eric,
The behavior on Mac is different from what you see. Clicking anywhere in a line's vertical indent space, moves the cursor to the beginning of the line below (not to that same position in the line below).

> I haven't tested on Windows yet, if someone with a Windows machine could try
> the snippet attached that would be appreciated.

I tested on Windows and notice the same behavior as you see on Linux.

IMO the vertical indent space of a line belongs to a line (below), so when I click anywhere in the vertical indent space I expect cursor to be placed on that line.

I tried a few things: Set line spacing in your example and clicked on it to see the behavior. It has similar behavior... the line spacing belongs to the line above it, so clicking on line spacing moves the cursor to the same position in line *above* it.
 
I also tested with MS Word and set line spacing, I see the same behavior as seen in StyledText on Windows & Linux. It doesn't have vertical indent, so can't say anything about that.
Even in this case behavior on Mac is different (strange?) and the cursor is placed at the end of the line!

I think the behavior on Mac is inconsistent with Windows and Linux for both vertical indent and line spacing and is a bug.

In the context of code mining the cursor moving looks wrong, clicking on the code mining should not change the cursor position. Should we have separate bug for it and fix it in code mining?
Comment 8 Eric Williams CLA 2018-12-06 10:51:55 EST
Thanks for the reply Lakshmi. I am fine with fixing this in code mining if we can all agree on that -- Angelo/Mickael please open up bugs against the relevant components and then we can close this one.
Comment 9 Eric Williams CLA 2019-02-20 14:20:11 EST
(In reply to Eric Williams from comment #8)
> Thanks for the reply Lakshmi. I am fine with fixing this in code mining if
> we can all agree on that -- Angelo/Mickael please open up bugs against the
> relevant components and then we can close this one.

Ping, are such tickets open, preferably one I can mark this as a dup of?
Comment 10 Eclipse Genie CLA 2021-02-10 01:32:51 EST
This bug hasn't had any activity in quite some time. Maybe the problem got resolved, was a duplicate of something else, or became less pressing for some reason - or maybe it's still relevant but just hasn't been looked at yet. As such, we're closing this bug.

If you have further information on the current state of the bug, please add it and reopen this bug. The information can be, for example, that the problem still occurs, that you still want the feature, that more information is needed, or that the bug is (for whatever reason) no longer relevant.

--
The automated Eclipse Genie.
Comment 11 Mickael Istria CLA 2021-02-10 03:02:44 EST
Mostly a dup of bug 570208 and family.

*** This bug has been marked as a duplicate of bug 570208 ***