Bug 289808 - [implementation] TextViewer.ViewerState.connect() sets fReverseSelection improperly
Summary: [implementation] TextViewer.ViewerState.connect() sets fReverseSelection impr...
Status: CLOSED DUPLICATE of bug 22592
Alias: None
Product: Platform
Classification: Eclipse Project
Component: Text (show other bugs)
Version: 3.4.2   Edit
Hardware: All All
: P5 minor (vote)
Target Milestone: ---   Edit
Assignee: Platform-Text-Inbox CLA
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2009-09-17 21:30 EDT by Scott Evans CLA
Modified: 2010-02-25 02:05 EST (History)
3 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Scott Evans CLA 2009-09-17 21:30:23 EDT
User-Agent:       Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.1.3) Gecko/20090824 Firefox/3.5.3
Build Identifier: 20090619-0625

Here's the line of code in question:

  fReverseSelection= selectionRange.y < 0;

But selectionRange can be tracked back to an API (see ViewerState.getSelection()) which is documented as:
  Returns the normalized selection, i.e. the the selection length is always non-negative.

This can cause unexpected behavior in client code when the user has a reverse selection. We see these in our own products; I've also noticed that certain JDT operations cause the selection to "flip" (like Correct Indentation and Shift Left/Right); that may be caused by this problem but I don't know.

Reproducible: Always

Steps to Reproduce:
Internal bug so no steps.
Comment 1 Dani Megert CLA 2009-09-18 04:34:11 EDT
I agree that the code looks wrong but unless I see it causing real harm I won't spend time to investigate and change it as it is like that for years.

Along with this TextViewer.getSelectedRange() does not 100% correctly implement the contract from ITextViewer.getSelectedRange() as it can return Point(-1,-1) in some case which isn't specified. That however, is also like that for years without anyone reporting a problem.


>I've also noticed that certain JDT
>operations cause the selection to "flip" (like Correct Indentation and Shift
>Left/Right)
That's by design and like that since day one i.e. long before that code was introduced.
Comment 2 Scott Evans CLA 2009-09-18 12:25:14 EDT
(In reply to comment #1)
> I agree that the code looks wrong but unless I see it causing real harm I won't
> spend time to investigate and change it as it is like that for years.
> 
> Along with this TextViewer.getSelectedRange() does not 100% correctly implement
> the contract from ITextViewer.getSelectedRange() as it can return Point(-1,-1)
> in some case which isn't specified. That however, is also like that for years
> without anyone reporting a problem.

We've seen some funny problems. For instance, when our app updates the annotations in an editor, if the user is dragging a selection upwards while the update happens, their selected text will suddenly change -- that's due to this. We may have others, it's hard to say.

I thought it'd be better for someone on the Text team to look into this as it's deep enough that I'd be worried about understanding the implications of fixing it.


> >I've also noticed that certain JDT
> >operations cause the selection to "flip" (like Correct Indentation and Shift
> >Left/Right)
>
> That's by design and like that since day one i.e. long before that code was
> introduced.

Huh, why would that be desirable behavior? I've always assumed it was a bug. The carat position shouldn't change as a result of an operation like that.
Comment 3 Dani Megert CLA 2009-09-18 12:55:16 EDT
>Huh, why would that be desirable behavior? 
I agree it's arguable but normally you expect to continue typing on the right of the selection. That's also how other text editors like UltraEdit behave.
Comment 4 Scott Evans CLA 2009-09-21 18:29:54 EDT
(In reply to comment #3)
> >Huh, why would that be desirable behavior? 
>
> I agree it's arguable but normally you expect to continue typing on the right
> of the selection. That's also how other text editors like UltraEdit behave.

I don't agree; I expect my cursor to stay put. 

A good example is to upwards-select more than a screenful of text, then press Tab to shift it right. Suddenly your cursor is gone.
Comment 5 Dani Megert CLA 2010-02-25 02:05:40 EST

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