Last Comment Bug 687093 - Clicking on a stack frame in the debugger should put the caret in the proper source line
: Clicking on a stack frame in the debugger should put the caret in the proper ...
Status: RESOLVED FIXED
:
Product: Firefox
Classification: Client Software
Component: Developer Tools: Debugger (show other bugs)
: Trunk
: All All
: P2 normal (vote)
: Firefox 13
Assigned To: Panos Astithas [:past]
:
Mentors:
Depends on: 687160 706506 726609
Blocks: minotaur
  Show dependency treegraph
 
Reported: 2011-09-16 09:53 PDT by Panos Astithas [:past]
Modified: 2012-02-13 08:20 PST (History)
3 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
WIP (5.15 KB, patch)
2012-01-27 07:04 PST, Panos Astithas [:past]
no flags Details | Diff | Splinter Review
WIP 2 (6.96 KB, patch)
2012-01-30 13:00 PST, Panos Astithas [:past]
no flags Details | Diff | Splinter Review
Working patch (15.09 KB, patch)
2012-01-31 09:33 PST, Panos Astithas [:past]
mihai.sucan: review+
Details | Diff | Splinter Review
Working patch v2 (15.11 KB, patch)
2012-02-01 06:44 PST, Panos Astithas [:past]
past: review+
Details | Diff | Splinter Review
[in-fx-team] Working patch v3 (16.13 KB, patch)
2012-02-10 08:54 PST, Panos Astithas [:past]
mihai.sucan: review+
Details | Diff | Splinter Review
Diff between v2 and v3 (1.72 KB, patch)
2012-02-10 08:55 PST, Panos Astithas [:past]
no flags Details | Diff | Splinter Review

Description Panos Astithas [:past] 2011-09-16 09:53:21 PDT
When a user clicks on a stack frame, the source editor pane should scroll the corresponding line into view (preferably at the top of the pane) and put the caret there. Highlighting the line would be nice, too.
Comment 1 Panos Astithas [:past] 2012-01-27 07:04:05 PST
Created attachment 592126 [details] [diff] [review]
WIP

Clicking on a stack frame now works as expected, but when entering the paused state the right script and line are not automatically selected. This cannot be fixed until bug 706506 is taken care of.
Comment 2 Panos Astithas [:past] 2012-01-30 13:00:26 PST
Created attachment 592834 [details] [diff] [review]
WIP 2

This works now (on top of bug 706506), but the test still has a couple of failures that I need to get to the bottom of.
Comment 3 Panos Astithas [:past] 2012-01-31 09:33:03 PST
Created attachment 593134 [details] [diff] [review]
Working patch

Fixed the test as well as a few others that this patch broke, and added an extra fix for a flicker during onScripts.
Comment 4 Mihai Sucan [:msucan] 2012-02-01 06:02:36 PST
Comment on attachment 593134 [details] [diff] [review]
Working patch

Review of attachment 593134 [details] [diff] [review]:
-----------------------------------------------------------------

Patch looks good!

::: browser/devtools/debugger/test/browser_dbg_select-line.js
@@ +69,5 @@
> +        });
> +      });
> +
> +      // Click the oldest stack frame.
> +      EventUtils.sendMouseEvent({ type: "click" },

Can you use synthesizeMouse() here?
Comment 5 Panos Astithas [:past] 2012-02-01 06:44:41 PST
Created attachment 593427 [details] [diff] [review]
Working patch v2

(In reply to Mihai Sucan [:msucan] from comment #4)
> Comment on attachment 593134 [details] [diff] [review]
> Working patch
> 
> Review of attachment 593134 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Patch looks good!
> 
> ::: browser/devtools/debugger/test/browser_dbg_select-line.js
> @@ +69,5 @@
> > +        });
> > +      });
> > +
> > +      // Click the oldest stack frame.
> > +      EventUtils.sendMouseEvent({ type: "click" },
> 
> Can you use synthesizeMouse() here?

Absolutely, fixed.

(carrying over the r+)
Comment 6 Panos Astithas [:past] 2012-02-10 08:54:56 PST
Created attachment 596060 [details] [diff] [review]
[in-fx-team] Working patch v3

Test fix required after the latest merge and a small hunk from the patch in bug 711164, since I reordered them. This hunk has extensive tests in that patch, but I wanted to get this in and not wait for stepping support to be finished.
Comment 7 Panos Astithas [:past] 2012-02-10 08:55:35 PST
Created attachment 596061 [details] [diff] [review]
Diff between v2 and v3

Changes since your last review.
Comment 8 Panos Astithas [:past] 2012-02-10 09:04:24 PST
Comment on attachment 596060 [details] [diff] [review]
[in-fx-team] Working patch v3

Sorry, I got confused. Mihai has been reviewing this.
Comment 9 Mihai Sucan [:msucan] 2012-02-10 09:57:22 PST
Comment on attachment 596060 [details] [diff] [review]
[in-fx-team] Working patch v3

Looks good!
Comment 10 Panos Astithas [:past] 2012-02-11 05:48:09 PST
https://hg.mozilla.org/integration/fx-team/rev/c17bad2abafe
Comment 11 Tim Taubert [:ttaubert] (on PTO, back Aug 29th) 2012-02-13 06:56:03 PST
https://hg.mozilla.org/mozilla-central/rev/c17bad2abafe

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