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

Bug 329050

Summary: Auto activation does not always work in javascript regions in webpages
Product: [WebTools] JSDT Reporter: Ian Tewksbury <itewksbu>
Component: WebAssignee: Ian Tewksbury <itewksbu>
Status: RESOLVED FIXED QA Contact: Nitin Dahyabhai <thatnitind>
Severity: normal    
Priority: P3 Flags: thatnitind: review+
Version: 3.2.3   
Target Milestone: 3.2.3   
Hardware: All   
OS: All   
Whiteboard: WI58566
Bug Depends on: 328366    
Bug Blocks:    
Attachments:
Description Flags
Patch - Option 1
none
Patch - Option 2
thatnitind: iplog+
additional patch
none
update patch none

Description Ian Tewksbury CLA 2010-10-29 10:09:46 EDT 
Bug 326602 was supposed to fix auto activation for JS in webpages.  Problem is that approach used overriding methods in StructuredTextViewerConfigurationJSDT and JSDTStructuredTextViewerConfigurationJSP.  The problem with doing that is that the StructuredTextViewerConfiguration extension point was not designed to have more then one StructuredTextViewerConfiguration provided for the same conetent type which is what happens when JSDT and SSE both provided a StructuredTextViewerConfiguration for the HTML and JSP content types.  Because of this the JSDT specific StructuredTextViewerConfiguration do not always get used.  This is dependent on load order, and while in WTP it tends to be the case that JSDT happens to load first it has been noticed in adopter products this is not always the case.

The fix for this is to use a new extension point being provided by the SSE team in  bug 328366.  This addition to the existing completionProposal extension point will allow JSDT to provide auto activation characters to be used in its partition types without the involvement of StructuredTextViewerConfigurations.
Comment 1 Ian Tewksbury CLA 2010-10-29 10:15:44 EDT 
Created attachment 182038 [details]
Patch - Option 1

Patch that takes advantage of the addition to the completionProposal extension point provided by Bug 328366.

This option one leaves the JSDT completionProposal extension point where it is in jsdt.web.ui but Nick pointed out that JSDT is referring to the JSP content type in that extension and that part of the extension should really be moved to our JSP plugin jsdt.web.support.jsp.  Thus I will be providing a second patch that both takes advantage of the new SSE changes and moves the JSP contribution to where it "should" be.
Comment 2 Ian Tewksbury CLA 2010-10-29 10:46:31 EDT 
Created attachment 182046 [details]
Patch - Option 2

As described in my last comment this patch moves the JSP JSDT content assist contribution to the jsdt.web.support.jsp.

Also in both patches I essentially reverted the initial change put in with Bug 326602. But moved the functionality of the existing SDTStructuredContentAssistProcessor to the new JSDTAutoActivationDelegate.
Comment 3 Ian Tewksbury CLA 2010-10-29 15:07:53 EDT 
Comment on attachment 182038 [details]
Patch - Option 1

We decided offline that it was a good idea to move the JSP content assist extension to the JSDT JSP plugin, thus leaving just option 2 up for review.
Comment 4 Nitin Dahyabhai CLA 2010-12-15 22:32:43 EST 
Created attachment 185286 [details]
additional patch

Didn't work until I made the attached changes.
Comment 5 Nitin Dahyabhai CLA 2010-12-15 22:58:23 EST 
Released.  Ian, please double-check the code changes when verifying.
Comment 6 Ian Tewksbury CLA 2010-12-16 07:19:38 EST 
Why are you using the super values before the local values?  What was the issue?  Last I tested this it worked.
Comment 7 Ian Tewksbury CLA 2010-12-16 07:41:18 EST 
Created attachment 185308 [details]
update patch

Oh, i see whats going on now, because SSE isn't implementing its own new AutoActivationDelegate and overriding the parent implementation.

Attaching a Patch to update JSDT to not override the functions it doesn't need to in JSDTStructuredContentAssistProcessor. And actually since there is nothing in there anymore, it may not even be needed anymore.