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

Bug 107320

Summary: StructuredTextEitor#setInput() creates another IStructuredModel
Product: [WebTools] WTP Source Editing Reporter: Hirotaka Matsumoto <jljlmatu>
Component: wst.sseAssignee: 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:
Description Flags
my trial patch none

Description Hirotaka Matsumoto CLA 2005-08-18 08:39:35 EDT
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.
Comment 1 Nitin Dahyabhai CLA 2005-08-19 13:43:03 EDT
Matsumoto-san, what IFile are you using to create the FileEditorInput, and what
is it's Path value?
Comment 2 Hirotaka Matsumoto CLA 2005-08-22 03:08:09 EDT
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
Comment 3 David Williams CLA 2005-08-31 22:36:58 EDT
I'm thinking this is related, if not dup of Bug 107316, but will investigate
after Bug 107316 is addressed. 
Comment 4 David Williams CLA 2005-09-22 13:32:01 EDT
Will give focus for M9. 
Comment 5 David Williams CLA 2005-11-20 23:48:04 EST
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. 
Comment 6 Hirotaka Matsumoto CLA 2005-11-22 07:34:57 EST
Created attachment 30371 [details]
my trial patch
Comment 7 Hirotaka Matsumoto CLA 2005-11-22 07:35:09 EST
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...















Comment 8 Hirotaka Matsumoto CLA 2005-11-23 22:27:46 EST
I've opened about FileBuffers#normalizeLocation's issue.
See https://bugs.eclipse.org/bugs/show_bug.cgi?id=117824


Comment 9 Hirotaka Matsumoto CLA 2005-11-24 01:00:40 EST
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... )

Comment 10 Nitin Dahyabhai CLA 2005-11-30 16:42:29 EST
Patch accepted, reviewed, and released.

And much appreciated.  Thanks, Matsumoto-san.
Comment 11 Hirotaka Matsumoto CLA 2006-06-19 23:22:43 EDT
Verified w/ WTP RC5
Comment 12 Hirotaka Matsumoto CLA 2006-06-19 23:22:56 EDT
 .