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

Bug 328639

Summary: [validation] JSP source validators still sometimes running even when respective batch validators have been turned off
Product: [WebTools] WTP Source Editing Reporter: Ian Tewksbury <itewksbu>
Component: wst.sseAssignee: Ian Tewksbury <itewksbu>
Status: RESOLVED FIXED QA Contact: Nitin Dahyabhai <thatnitind>
Severity: normal    
Priority: P3 CC: ccc, keith.chong.ca, nsand.dev
Version: 3.0.5Flags: nsand.dev: review+
thatnitind: review+
Target Milestone: 3.0.5 P   
Hardware: All   
OS: All   
Whiteboard:
Attachments:
Description Flags
Fix Patch
none
Fix Patch - Update 1
none
Fix Patch - Update 2 ccc: iplog+

Description Ian Tewksbury CLA 2010-10-25 13:12:05 EDT
This bug is a continuation of Bug 320322.  There are still some cases that are broken.

CASE 1:
0. global enable all validators
1. global preference enabled to allow project specific settings
2. project specific settings enabled for a project
3. disable validators using project specific settings
4. disabled global preference to allow project specific settings

in this case ValidatorStrategy is only checking the project specific settings and does not check if the global enablement of project specific settings.

CASE 2:
0. global enable all validators
1. no project specific settings
2. start causing a validation error in a file
3. do NOT save the file
4. an as you type error annotation will be added
5. still WITHOUT saving open the global validation preferences and disable all validators

In this case any further typing in the unsaved document will not add further as you type validation annotations but any existing as you type validation annotations will not go away.
Comment 1 Ian Tewksbury CLA 2010-10-25 13:13:27 EDT
Created attachment 181665 [details]
Fix Patch

Patch to fix the issues mentioned in the description.
Comment 2 Nick Sandonato CLA 2010-10-25 17:37:19 EDT
Ian, the thing that concerns me is that this blows away all annotations. So, for example, you'll lose spelling annotations. Spelling might be small, but there may be other--more important--annotations added by contributors to the editor.
Comment 3 Ian Tewksbury CLA 2010-10-26 08:44:15 EDT
Created attachment 181715 [details]
Fix Patch - Update 1

Bummer, didn't realize it was a shared model like that.

This new patch iterates over all of the existing annotations and removes only those of type:

TemporaryAnnotation.ANNOT_ERROR
TemporaryAnnotation.ANNOT_WARNING
TemporaryAnnotation.ANNOT_INFO

Where TemporaryAnnotation is an SSE class.  In this way I hope I am only removing the annotations that will be added back if the validator is turned back on.  I tested this and at least spelling markers now stay.

My only worry is if these TemporaryAnnotation are used for something else that this will now blow away.  Though if that is the case I don't see any other identifying information on the TemporaryAnnotation that ValidatorStrategy could use to know which ones to remove.  Not saying such info doesnt exist, or that such info couldn't be added, I just don't see anything else.
Comment 4 Nick Sandonato CLA 2010-10-26 09:33:09 EDT
Ian, there were several cases where I disabled all validators, globally or at the project level, where annotations were being generated after typing in the editor. I didn't notice this problem before the patch. Can you investigate if this is a result of the changes?

Also, I don't think you need to clean up the ANNOT_INFO markers.
Comment 5 Ian Tewksbury CLA 2010-10-26 11:24:20 EDT
Created attachment 181738 [details]
Fix Patch - Update 2

(In reply to comment #4)
> Ian, there were several cases where I disabled all validators, globally or at
> the project level, where annotations were being generated after typing in the
> editor. I didn't notice this problem before the patch. Can you investigate if
> this is a result of the changes?

That is caused by issues in the Val framework.  The Val framework team is currently working on a patch to fix that up.

> Also, I don't think you need to clean up the ANNOT_INFO markers.

I did this by taking it a step further.  As we discussed only the annotations for the disabled validators used by the ValStrat should be removed.  To this end I updated the patch to determine which validators are disabled and associated with the ValStrat and then only remove the annotations associated with those validators.
Comment 6 Keith Chong CLA 2010-11-11 09:34:51 EST
(In reply to comment #5)
> Created an attachment (id=181738) [details]
> Fix Patch - Update 2
> (In reply to comment #4)
> > Ian, there were several cases where I disabled all validators, globally or at
> > the project level, where annotations were being generated after typing in the
> > editor. I didn't notice this problem before the patch. Can you investigate if
> > this is a result of the changes?
> That is caused by issues in the Val framework.  The Val framework team is
> currently working on a patch to fix that up.
> > Also, I don't think you need to clean up the ANNOT_INFO markers.
> I did this by taking it a step further.  As we discussed only the annotations
> for the disabled validators used by the ValStrat should be removed.  To this
> end I updated the patch to determine which validators are disabled and
> associated with the ValStrat and then only remove the annotations associated
> with those validators.

See bug 329997, against the VF.
Comment 7 Nick Sandonato CLA 2010-11-11 11:03:41 EST
Patch looks good. Thanks, Ian.
Comment 8 Carl Anderson CLA 2010-11-17 16:23:02 EST
Committed to R3_0_5_patches