| 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: | Text | Assignee: | 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: |
|
||||||
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 on attachment 155900 [details]
Possible patch that leaves -1 except for 0 length case
The patch is wrong as it assigns -1 to end.
Fixed in HEAD. Available in builds > N20100112-2000. (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. |
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.