Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.

Bug 299416

Summary: AnnotationModel#isWithinRegion does not work for regions of the same offset as the region with a length of 0
Product: [Eclipse Project] Platform Reporter: Ian Tewksbury <itewksbu>
Component: TextAssignee: Dani Megert <daniel_megert>
Status: RESOLVED FIXED QA Contact:
Severity: normal    
Priority: P3 CC: daniel_megert
Version: 3.6   
Target Milestone: 3.6 M5   
Hardware: All   
OS: All   
Whiteboard:
Attachments:
Description Flags
Possible patch that leaves -1 except for 0 length case daniel_megert: review-

Description Ian Tewksbury CLA 2010-01-12 14:33:45 EST
Created attachment 155900 [details]
Possible patch that leaves -1 except for 0 length case

AnnotationModel#isWithinRegion does not work for regions of the same offset as the region with a length of 0 because both the fCanStartBefore and default conditions have the check:

fRegion.includes(start + length - 1);

which means the calculated location will be one less then the given start.  Not sure why this -1 is in there in the first place, but if it needs to stay maybe it could be changed to something like the supplied patch.
Comment 1 Dani Megert CLA 2010-01-13 05:17:53 EST
Agree. Can you provide a test case that shows what this breaks, e.g. something bad happening in the UI, so that we can verify the fix?
Comment 2 Dani Megert CLA 2010-01-13 05:31:50 EST
Comment on attachment 155900 [details]
Possible patch that leaves -1 except for 0 length case

The patch is wrong as it assigns -1 to end.
Comment 3 Dani Megert CLA 2010-01-13 05:37:49 EST
Fixed in HEAD.
Available in builds > N20100112-2000.
Comment 4 Ian Tewksbury CLA 2010-01-13 08:31:54 EST

(In reply to comment #1)
> Agree. Can you provide a test case that shows what this breaks, e.g. something
> bad happening in the UI, so that we can verify the fix?

I ran across this with the code folding implemented in WTP.  In the implimentation whenever a region changes I be sure to check for any annotations that now have a length of 0 so they can be removed, but in some cases this annotation was at the beginning of a region and thus would not be returned and then not removed so a 0 length annotation would end up hanging around in the model.  I only caught this because of some JUnits I wrote.  So nothing in the UI to verify against, but I can report back if I see the problem fixed in my area.  

(In reply to comment #2)
> (From update of attachment 155900 [details])
> The patch is wrong as it assigns -1 to end.

oops, got my -= backwards.

Thanks for the quick response.