Bug 149308

Comment 1 Nitin Dahyabhai 2006-07-05 03:20:38 EDT

Is a batch validator registered, enabled, and running on save?

Comment 2 Adam Peller 2006-07-06 22:49:57 EDT

I investigated a bit more.  The validator(s) are indeed registered and running.  The problem is not unique to Linux.  Here is the code path:

org.eclipse.atf.javascript.validator,org/eclipse/atf/javascript/validator/internal/validation/JSAbstractValidator.java:
validate()->validateDelta()->validateFile()->validateDOM()

This is a piece of code I hoped I'd never have to revisit again.  Finding the regions in an HTML document which represent SCRIPT is non-trivial.  I had asked that it be wrapped up in an API (bug #120963) not just because I'm lazy, but because I felt the approach I ended up taking looked very fragile.  Also because I'd like additional support, like finding script in tag attributes.  I could be reading too much into this, and it could be something I did, but I don't think I've touched this code in some time and it seems to have regressed recently.  There seems to be too much knowledge of internal WTP in this routine.

At org.eclipse.atf.javascript.validator,org/eclipse/atf/javascript/validator/internal/validation/JSAbstractValidator.java:316 I'm iterating through regions and partitions looking for a match on content type of org.eclipse.wst.html.SCRIPT.  All I see are org.eclipse.wst.html.HTML_DEFAULT, so the script never gets validated in batch mode.

David/Nitin, let me know if you're aware of any changes in 1.5 which might have tripped me up.  Also, let me know if you think formalizing this in an API might be a good idea.

Comment 3 Nitin Dahyabhai 2006-07-07 03:14:35 EDT

Adam, I don't think the HTML partitioner in WTP has ever properly detected JavaScript within HTML attributes--at runtime I expect it would be terribly slow.  It should correctly detect the contents between script start and end tags, though.  You could probably get a good performance boost from asking the partitioner for all of the partitions in the document from the start, rather than iterating through each text region.  From there you would check which partitions have the type for scripts.  I doubt that you need to be as invested in the document's structure as that method is to do what you're trying to do.

Comment 4 Adam Peller 2006-07-07 10:11:35 EDT

This code definitely worked until one of the 1.5RC releases, though like I said I was never very comfortable with it.

Nitin, I've been back and forth on this and just don't know the APIs.  Can you help me figure out how to fix this code for 1.5?  Performance improvements are certainly nice, but anything that works for now is good.  And David, can we raise the urgency of #120963 to basically put the solution behind a method in the WTP so it can be supported for others to use?  Of course, if you guys wrote the method for 120963, I could temporarily back port it to support 1.5 :-)

Comment 5 Adam Peller 2006-07-20 22:48:08 EDT

Can anyone help here?

Comment 6 David Williams 2006-07-20 23:25:01 EDT

(In reply to comment #5)
> Can anyone help here?
> 

Yep. I suggest assigning this to WTP, javascript component. 
How, exactly, is ATF's javascript component different/similar to WTP's javascript component? Guess that's one of the things to "merge"? 

Comment 7 Adam Peller 2006-07-21 00:10:31 EDT

Thanks for looking at this.  As it's ATF code that ultimately needs to be fixed, I think the right course of action might be to address bug #120963 and then just have the ATF code call that new API (though we will probably need to retrofit something to support WTP 1.5)  I've raised severity to "major", though you might say it's blocking our release (um, assuming we had a concrete release schedule :)

The ATF javascript component was poorly named.  It really just represents the JavaScript validator extension we've written.  I had originally called it something closer to your naming convention like org.eclipse.atf.validator.javascript which got shortened to just org.eclipse.atf.javascript, which makes the component sound like a lot more than it is!  (Well, maybe someday it will do more?)

Whether all of this validator code belongs in the WTP is another matter.  Given that we are still using a large number of internal APIs likely to change or break this code, perhaps that's a good idea.  The reason we didn't do this initially was because of our use of 3rd party software such as Rhino and JSLint.

Comment 8 Adam Peller 2006-07-21 00:11:14 EDT

Thanks for looking at this.  As it's ATF code that ultimately needs to be fixed, I think the right course of action might be to address bug #120963 and then just have the ATF code call that new API (though we will probably need to retrofit something to support WTP 1.5)  I've raised severity to "major", though you might say it's blocking our release (um, assuming we had a concrete release schedule :)

The ATF javascript component was poorly named.  It really just represents the JavaScript validator extension we've written.  I had originally called it something closer to your naming convention like org.eclipse.atf.validator.javascript which got shortened to just org.eclipse.atf.javascript, which makes the component sound like a lot more than it is!  (Well, maybe someday it will do more?)

Whether all of this validator code belongs in the WTP is another matter.  Given that we are still using a large number of internal APIs likely to change or break this code, perhaps that's a good idea.  The reason we didn't do this initially was because of our use of 3rd party software such as Rhino and JSLint.

Comment 9 Robert Goodman 2006-07-31 21:25:24 EDT

I dropped a change that seems to work around the problem. Adam please verify the change. 

Comment 10 Robert Goodman 2007-08-30 14:28:59 EDT

These Javascript validators have been replaced by the validators in JSDT and it looks like I drop some work around for the problem. I'm closing the bud since it is not lonager valid. The validators are no longer used.