Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.
Bug 354283 - Refresh highlights wrong tab when two files with the same absolute path are open
Summary: Refresh highlights wrong tab when two files with the same absolute path are open
Status: RESOLVED FIXED
Alias: None
Product: Target Management
Classification: Tools
Component: RSE (show other bugs)
Version: 3.3   Edit
Hardware: PC Linux
: P3 normal (vote)
Target Milestone: 3.3.1   Edit
Assignee: David McKnight CLA
QA Contact: Martin Oberhuber CLA
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-08-09 13:51 EDT by David I CLA
Modified: 2011-08-12 10:45 EDT (History)
2 users (show)

See Also:
mober.at+eclipse: pmc_approved+
xuanchen: review+

Attachments
patch to compare subsystems (4.96 KB, patch)
2011-08-10 10:49 EDT, David McKnight CLA 		no flags Details | Diff
updated patch with extra null check on the subsystems (5.00 KB, patch)
2011-08-10 12:44 EDT, David McKnight CLA 		no flags Details | Diff
Show Obsolete (1) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description David I CLA 2011-08-09 13:51:21 EDT 
Open the same filename on two different remote hosts using SSH. Edit and save the second one, and a refresh event is generated. However, the first tab with that filename is highlighted and gains focus. The first tab may not be the one you were working with.
Comment 1 Martin Oberhuber CLA 2011-08-10 04:42:23 EDT 
Was it only the same file name (in different folders), or was it the same absolute path?

Dave M: You've been working on Refresh before, what do you think?
Comment 2 David McKnight CLA 2011-08-10 10:25:35 EDT 
(In reply to comment #1)
> Was it only the same file name (in different folders), or was it the same
> absolute path?
> 
> Dave M: You've been working on Refresh before, what do you think?

I recreated the scenario by creating two files with identical paths on their respective systems.  When the "link with editor" is off or the paths are not identical the problem does not occur.
Comment 3 David McKnight CLA 2011-08-10 10:49:22 EDT 
Created attachment 201247 [details]
patch to compare subsystems

The original code checks for a remote path associated with the IFile that matches the corresponding remote object but it doesn't check the subsystems.  This patch addresses that.
Comment 4 David McKnight CLA 2011-08-10 10:57:29 EDT 
Martin and Xuan could you please review this patch?
Comment 5 Martin Oberhuber CLA 2011-08-10 11:13:26 EDT 
Can objSS == null ? Problebly not since adapter.getAbsoluteName(obj)!=null ?

If it can be null, then you might want to do this in line 431:

if (objSS==null || objSS.equals(editorSS)) {
    page.bringToTop(editor);
    return;
}
Comment 6 David McKnight CLA 2011-08-10 11:38:20 EDT 
(In reply to comment #5)
> Can objSS == null ? Problebly not since adapter.getAbsoluteName(obj)!=null ?
> 
> If it can be null, then you might want to do this in line 431:
> 
> if (objSS==null || objSS.equals(editorSS)) {
>     page.bringToTop(editor);
>     return;
> }


I wouldn't think objSS could be null since the adapter does return a path for the object but it never hurts to add a null check.
Comment 7 David McKnight CLA 2011-08-10 12:44:26 EDT 
Created attachment 201257 [details]
updated patch with extra null check on the subsystems
Comment 8 David I CLA 2011-08-10 13:55:23 EDT 
You are correct that the path names were the same
Comment 9 Martin Oberhuber CLA 2011-08-11 07:09:04 EDT 
+1 I'm fine with the patch.

The "editorSS != null" is not needed on line 431 since equals() deals with that.
BTW since we're now under the Tools PMC, I'm not a PMC member any more.
Comment 10 David McKnight CLA 2011-08-11 10:03:39 EDT 
I've committed the change to cvs although I'm still waiting on the review by Xuan.
Comment 11 Xuan Chen CLA 2011-08-12 10:45:43 EDT 
Sorry, I was away yesterday.

The fix looks good.  Thanks.