| Summary: | StructuredTextEitor#setInput() creates another IStructuredModel | ||||||
|---|---|---|---|---|---|---|---|
| Product: | [WebTools] WTP Source Editing | Reporter: | Hirotaka Matsumoto <jljlmatu> | ||||
| Component: | wst.sse | Assignee: | Nitin Dahyabhai <thatnitind> | ||||
| Status: | CLOSED FIXED | QA Contact: | |||||
| Severity: | critical | ||||||
| Priority: | P2 | CC: | david_williams, ryman, sisikawa | ||||
| Version: | 0.7 | ||||||
| Target Milestone: | 1.0 M10 | ||||||
| Hardware: | PC | ||||||
| OS: | Windows XP | ||||||
| Whiteboard: | |||||||
| Attachments: |
|
||||||
Matsumoto-san, what IFile are you using to create the FileEditorInput, and what is it's Path value? the implementation of IFile is org.eclipse.core.internal.resources.File which was obtained by IProject#getFile api. Also inside of this class, the path is /dynamicWeb/WebContent/frame/a15.html I'm thinking this is related, if not dup of Bug 107316, but will investigate after Bug 107316 is addressed. Will give focus for M9. I'm changing milestone to acknowledge I did not get this done for M9, but do still believe its important, and will attempt to find a safe fix to put in before R1.0. Thanks very much. Created attachment 30371 [details]
my trial patch
When FileEditorInput is passed to StructuredTextEditor#doSetInput,
we will go through the following stack trace :
TextFileDocumentProvider.createFileInfo(Object) line: 536
TextFileDocumentProvider.connect(Object) line: 463
StructuredTextEditor(AbstractTextEditor).doSetInput(IEditorInput) line: 3008
StructuredTextEditor(StatusTextEditor).doSetInput(IEditorInput) line: 173
StructuredTextEditor(AbstractDecoratedTextEditor).doSetInput(IEditorInput) line: 1303
StructuredTextEditor(TextEditor).doSetInput(IEditorInput) line: 262
StructuredTextEditor.doSetInput(IEditorInput) line: 1645
and StructuredTextEditor#doSetInput eventually calls TextFileDocumentProvider#createFileInfo.
Its snippet code is :
protected FileInfo createFileInfo(Object element) throws CoreException {
IPath location= null;
if (element instanceof IAdaptable) {
IAdaptable adaptable= (IAdaptable) element;
ILocationProvider provider= (ILocationProvider) adaptable.getAdapter(ILocationProvider.class);
if (provider != null)
location= provider.getPath(element);
}
if (location != null) {
ITextFileBufferManager manager= FileBuffers.getTextFileBufferManager();
manager.connect(location, getProgressMonitor());
In this timing, because FileEditorInput is passed as element and its getAdapter(ILocationProvider.class)
returns FileEditorInputAdapterFactory of which getPath(FIleEditorInput) returns
FileEditorInput#getFile#getFullPath(). So 'fullpath', such as
"/ProjectName/WebContent/jsf1.jsp" is passed to TextFileBufferManager#connect.
On the other hand, when IFile is passed to ModelManagerImpl#getModelForEdit,
we will get into the following stack trace :
FileBufferModelManager.getModel(IFile) line: 504
ModelManagerImpl._commonGetModel(IFile, String, IModelHandler, URIResolver, ModelManagerImpl$ReadEditType, String, String) line: 318
ModelManagerImpl._commonGetModel(IFile, ModelManagerImpl$ReadEditType, String, String) line: 304
ModelManagerImpl.getModelForEdit(IFile) line: 998
and FileBufferModelManager#getModel is eventually called. Its code is :
IStructuredModel model = null;
ITextFileBufferManager bufferManager = FileBuffers.getTextFileBufferManager();
try {
if (Logger.DEBUG_FILEBUFFERMODELMANAGEMENT) {
System.out.println("FileBufferModelManager connecting to IFile " + file.getLocation()); //$NON-NLS-1$
}
bufferManager.connect(file.getLocation(), getProgressMonitor());
Here, IFile#getLocation() is passed to TextFileBufferManager#connect. The point is in this case,
'location', such as "c:/workspace/ProjectName/WebContent/jsf1.jsp" is passed to.
The key is that if TextFileBufferManager#connect is called by StructuredTextEditor.doSetInput,
IFile#getFullPath is passed whereas if it is called by ModelManagerImpl#getModelForEdit,
IFile#getLocation is passed. If this IFile represents for the existing file, this difference
doesn't cause any problem because the magic is in FileBuffers#normalizeLocation. This code is
public static IPath normalizeLocation(IPath pathOrLocation) {
IWorkspaceRoot workspaceRoot= ResourcesPlugin.getWorkspace().getRoot();
// existing workspace resources - this is the 93% case
if (workspaceRoot.exists(pathOrLocation))
return pathOrLocation.makeAbsolute();
IFile file= workspaceRoot.getFileForLocation(pathOrLocation);
// existing workspace resources referenced by their file system path
// files that do not exist (including non-accessible files) do not pass
if (file != null && file.exists())
return file.getFullPath();
// non-existing resources and external files
return pathOrLocation.makeAbsolute();
}
If IFile represents for the existing file, both forms returns "
file's workspace relative, absolute path as returned by IFile#getFullPath()" (Please
see java doc for above api ). However, if IFile represents for non existing file,
the return value from this api with IFile#getFullPath and the return value with
IFile#getLocation is different. ( In the above example, "/ProjectName/WebContent/jsf1.jsp"
vs "c:/workspace/ProjectName/WebContent/jsf1.jsp") So my first suggested solution would be to change
FileBufferModelManager#getModel to use IFile#getFullPath, not IFile#getLocation. ( and
FileBufferModelManager#calculateId as well. Please see patch file. I may think
this is a little bit strange and this could be FileBuffers#normalizeLocation's bug, though. )
While I tested this code for a few seconds, it seems to work fine, but when I took around the
other code, you may assume that IFileBuffer.getLocation() always returns 'location'( i.e.
c:/workspace/ProjectName/WebContent/jsf1.jsp ), if so, probably this doesn't work. If you
believe so, another solution would be that as TextEditor does, we instantiate 'NonExistingFileEditorInput'
for the non existing file, so that if our NonExistingFileEditorInput#getAdapter(ILocationProvider.class)#getPath
returns 'location', not a 'fullpath', it also works. ( We tested this way for a few seconds, and
it worked too. ) At this moment, ours returns 'fullpath' ( because FileEditorInput returns 'fullpath',
not 'location' ) so I'm little bit concerned we may return 'location' or not.
Well, another soltuion would be to change ModelManagerImpl._commonGetModel not to use
FileBufferModelManager if passed IFile doesn't exist, instead, it needs to use your own way as
you do for InputStream. ( Of course, StructuredTextEditor also needs to be changed to use the same
way if non existing IEditorInput is passed to. ). but this choice would be very large changes...
I've opened about FileBuffers#normalizeLocation's issue. See https://bugs.eclipse.org/bugs/show_bug.cgi?id=117824 Please see https://bugs.eclipse.org/bugs/show_bug.cgi?id=107316. The same patch will fix this bug as well. I understand that the proposed solution highly depends on how FileBuffers#normalizeLocation and IPath#makeAbsolute behave, but I don't think their behaviour will be changed ( because lots of component may depend this behaviour already... ) Patch accepted, reviewed, and released. And much appreciated. Thanks, Matsumoto-san. Verified w/ WTP RC5 . |
StructuredTextEitor#setInput() creates another IStructuredModel even if the same IFile is passed to. Given that we have the following pseudo code : IFile newFile; // This represents for the non existing file, but points to the // inside of the workspace // let's say C:/workspace/dynamicWeb/WebContent/frame/a15.html IStructuredModel newModel = IModelManager#getNewModelForEdit(newFile); // In this timing, newModel's id is // C:/workspace/dynamicWeb/WebContent/frame/a15.html StructuredTextEditor#setInput(new FileEditorInput(newFile)); when we reach the StructuredTextEditor#doSetInput, it creates another IStructuredModel and uses it. What is expected is that the same instance of newModel should be used there. This prevents our jsp/html editor from sync-ing with your source editor when a frame document is newly created. ( so that the modification which is made from our editor isn't propagated to the model in the source editor. ) This is also critical.