Last Comment Bug 734911 - Adding breakpoints sometimes doesn't work
: Adding breakpoints sometimes doesn't work
Status: RESOLVED FIXED
:
Product: Firefox
Classification: Client Software
Component: Developer Tools: Debugger (show other bugs)
: Trunk
: All All
: P2 normal (vote)
: Firefox 14
Assigned To: Panos Astithas [:past]
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2012-03-12 09:50 PDT by Panos Astithas [:past]
Modified: 2012-03-29 02:56 PDT (History)
2 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Fix (1.05 KB, patch)
2012-03-19 11:54 PDT, Panos Astithas [:past]
rcampbell: review+
Details | Diff | Splinter Review

Description Panos Astithas [:past] 2012-03-12 09:50:28 PDT
STR:

1) Load http://htmlpad.org/debugger/ and open the script debugger
2) Reload page.
3) Click on 'Click me!'
3) Try to add a breakpoint in line 19 or 20: noScript error is returned from the protocol

It may be a bug in the innermost script calculation or some issue with the workaround for the lack of findScripts.
Comment 1 Mihai Sucan [:msucan] 2012-03-12 10:02:32 PDT
I noticed this while writing the test for the breakpoints source editor integration stuff.

I reliably reproduced the issue and it happened on certain lines of code only. For example if I tried to add a breakpoint where a comment is I get the error.

The line numbers I picked first were giving back such errors. I had to see which lines in the code don't give the error. I determined it doesn't have to be whitespace or a comment-only line. This was surprising because when I use the debugger myself, I can add breakpoints wherever I want.
Comment 2 Panos Astithas [:past] 2012-03-12 10:13:39 PDT
(In reply to Mihai Sucan [:msucan] from comment #1)
> I noticed this while writing the test for the breakpoints source editor
> integration stuff.
> 
> I reliably reproduced the issue and it happened on certain lines of code
> only. For example if I tried to add a breakpoint where a comment is I get
> the error.
> 
> The line numbers I picked first were giving back such errors. I had to see
> which lines in the code don't give the error. I determined it doesn't have
> to be whitespace or a comment-only line. This was surprising because when I
> use the debugger myself, I can add breakpoints wherever I want.

Your case may be a slightly different issue: the findScripts workaround doesn't work for inline scripts AFAICT, because these notifications come before the debugger client is ready to listen for them. This is a common pattern in mochitests, so I've reverted to using eval, in order for scripts to be discovered way after the debugger is initialized. In my STR above, if you remove step 2, no breakpoint can be set outside of the eval script.

You can verify from the protocol log if there is a script found containing the line you are trying to set a breakpoint to, or provide me with STR to verify it myself.
Comment 3 Panos Astithas [:past] 2012-03-12 10:30:44 PDT
(In reply to Panos Astithas [:past] from comment #2)
> Your case may be a slightly different issue: the findScripts workaround
> doesn't work for inline scripts AFAICT, because these notifications come
> before the debugger client is ready to listen for them. This is a common

Er, what I meant to say here is that the workaround doesn't work without a page reload, which we don't commonly do in mochitests.
Comment 4 Mihai Sucan [:msucan] 2012-03-12 13:10:12 PDT
Ah, that makes sense. I forgot to check if a page reload would fix that. Thanks Panos!
Comment 5 Panos Astithas [:past] 2012-03-19 11:54:57 PDT
Created attachment 607238 [details] [diff] [review]
Fix

Embarrassingly enough, this is the fix and I'm the one to blame here. I'll see about adding a test case.
Comment 6 Panos Astithas [:past] 2012-03-20 06:01:19 PDT
Comment on attachment 607238 [details] [diff] [review]
Fix

So, adding a test seems to be too much work for such an obvious error (I should have spotted it from reading the comment above, for crying out loud!).
Comment 7 Rob Campbell [:rc] (:robcee) 2012-03-27 06:56:57 PDT
Comment on attachment 607238 [details] [diff] [review]
Fix

I thought I already r+'d this. Sorry for the delay!
Comment 8 Panos Astithas [:past] 2012-03-28 04:13:25 PDT
https://hg.mozilla.org/integration/fx-team/rev/86907cf92dae
Comment 9 Tim Taubert [:ttaubert] 2012-03-29 02:56:38 PDT
https://hg.mozilla.org/mozilla-central/rev/86907cf92dae

Note You need to log in before you can comment on or make changes to this bug.