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

Bug 354283

Summary:

Refresh highlights wrong tab when two files with the same absolute path are open

Product:

[Tools] Target Management

Reporter:

David I <david.ishee>

Component:

RSE

Assignee:

David McKnight <dmcknigh>

Status:

RESOLVED FIXED

QA Contact:

Martin Oberhuber <mober.at+eclipse>

Severity:

normal

Priority:

CC:

dmcknigh, xuanchen

Version:

3.3

Flags:

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

Target Milestone:

3.3.1

Hardware:

OS:

Linux

Whiteboard:

Attachments:

Description	Flags
patch to compare subsystems	none
updated patch with extra null check on the subsystems	none

Description David I

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

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

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

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

2011-08-10 10:57:29 EDT

Martin and Xuan could you please review this patch?

Comment 5 Martin Oberhuber

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

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

2011-08-10 12:44:26 EDT

Created attachment 201257 [details]
updated patch with extra null check on the subsystems

Comment 8 David I

2011-08-10 13:55:23 EDT

You are correct that the path names were the same

Comment 9 Martin Oberhuber

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

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

2011-08-12 10:45:43 EDT

Sorry, I was away yesterday.

The fix looks good.  Thanks.