Bug 313013

Comment 1 jdance 2010-08-31 17:43:53 EDT

Here is an additional example.  The inability to set breakpoints in function literals severely restricts debugging functionality in JSDT.

Start with the following JavaScript. Breakpoints can be set on any valid line in the toggle function.

var state = 'hello';

function toggle()
{
	var helloElem = document.getElementById("hello");
	var hello_againElem = document.getElementById("hello_again");
	
	if(state == 'hello')
	{
		helloElem.style.display = 'none';
		hello_againElem.style.display = 'block';
		state='hello_again';
	}
	else
	{
		helloElem.style.display = 'block';
		hello_againElem.style.display = 'none';
		state='hello';
	}
}

Now change toggle to a function literal (shown below):

var state = 'hello';

toggle = function()
{
	var helloElem = document.getElementById("hello");
	var hello_againElem = document.getElementById("hello_again");
	
	if(state == 'hello')
	{
		helloElem.style.display = 'none';
		hello_againElem.style.display = 'block';
		state='hello_again';
	}
	else
	{
		helloElem.style.display = 'block';
		hello_againElem.style.display = 'none';
		state='hello';
	}
};

No breakpoints can be set anywhere in the function.  The status line reports "A valid location could not be found for the breakpoint".

Comment 2 Michael Rennie 2010-09-02 15:03:24 EDT

Created attachment 178091 [details]
partial fix

The attached patch fixes setting breakpoints in BodyDeclarations like the examples given, but I encountered some curious behavior when trying to compute a backing IJavaScriptElement.

For example, if I try to find the IJSE for the following snippet it will always return null when I call ITypeRoot.getElementAt(offset) when I pass in the offset for the start of the function() decl. Nitin, shouldn't it return an IFunction? Event if I compute the offset to be within the body of a function decl of this form (provided it has a body) ITypeRoot.getElementAt(offset) will return null.

var v1 = "1";
foo = function() {
  var iv = "2";
}

Comment 3 jdance 2010-09-02 19:06:28 EDT

Is it possible to get this patch integrated into the SR1 release?  If not, is it possible to get the location verifier turned off until this (and related bug #324144) get fixed?

We have integrated JSDT with ChromeDevTools, and have users trying to be productive with this environment. The bugs in the location verifier, which prevent setting breakpoints at valid locations is a big showstopper. Allowing breakpoints at non-executable lines would be the lesser of the two evils.  Thanks!

Comment 4 Michael Rennie 2010-09-03 10:14:11 EDT

(In reply to comment #3)
> Is it possible to get this patch integrated into the SR1 release? 

Sadly no, SR1 will be shipping next week, so it is too late for any more fixes.

> If not, is
> it possible to get the location verifier turned off until this (and related bug
> #324144) get fixed?

There is no way I can think of to turn it off, either fix - 'turning it off' or applying the patch - would require you to move up to the next I build of JSDT to have a fix.

> We have integrated JSDT with ChromeDevTools, 

That sounds very cool, we also have a project to integrate with ChromeDevTools, although ours is nothing more than a stub with no packet implementations yet.

http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.jsdt/development/org.eclipse.wst.jsdt.debug.chrome/?root=WebTools_Project

> The bugs in the location verifier, which
> prevent setting breakpoints at valid locations is a big showstopper. Allowing
> breakpoints at non-executable lines would be the lesser of the two evils. 
> Thanks!

I'll do my best to get the verifier up to snuff. Thanks for the test cases - if you have any more please post them. I will also port over the breakpoint test framework from platform debug so we can automatically find these issues.

Comment 5 Michael Rennie 2010-09-03 10:41:37 EDT

applied patch to HEAD, will wait to mark fixed until I get the test suite up and running

Comment 6 Michael Rennie 2010-09-03 16:16:55 EDT

Created attachment 178203 [details]
abstract test

This patch contains an abstract debug test with methods to create breakpoints / open editors / toggle breakpoints / work with test source...now all thats left is writing the tests.

Comment 7 Michael Rennie 2010-09-28 13:01:34 EDT

applied the test fixes to HEAD. Marking this bug fixed, any new bugs can be logged separately.