Last Comment Bug 659910 - Evaluating console.log or other API messages in Scratchpad presents wrong source link in web console
: Evaluating console.log or other API messages in Scratchpad presents wrong sou...
Status: VERIFIED FIXED
[scratchpad][console][view-source][fi...
:
Product: Firefox
Classification: Client Software
Component: Developer Tools (show other bugs)
: unspecified
: All All
: -- normal (vote)
: Firefox 6
Assigned To: Panos Astithas [:past] (away until 7/21)
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2011-05-26 05:47 PDT by Rob Campbell [:rc] (:robcee)
Modified: 2011-06-16 05:04 PDT (History)
5 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
-
fixed


Attachments
[in-devtools][checked-in] Simple fix (1011 bytes, patch)
2011-05-27 07:01 PDT, Panos Astithas [:past] (away until 7/21)
rcampbell: review+
gavin.sharp: review+
gavin.sharp: approval‑mozilla‑aurora+
Details | Diff | Splinter Review

Description Rob Campbell [:rc] (:robcee) 2011-05-26 05:47:56 PDT
Steps:

1. Open a Scratchpad (F4, Web Developer menu)
2. Enter console.log("hello");
3. Open the Web Console (Cmd/Ctrl-Shift-K from Scratchpad)
4. Select text console.log("hello");, right click and select Run
5. Verify that the console output appears with a source link to the right that reads "Scratchpad:1"
6. Click the source link

Expected Results:
Either a view source window appears with the contents of the evaluated Scratchpad text or the Scratchpad window could be refocused.

Actual results:

View Source window with the contents of Scratchpad.com are shown.
Comment 1 Kevin Dangoor 2011-05-26 06:52:45 PDT
If the patch for this is not risky-looking, we should nominate this for Firefox 6.
Comment 2 Panos Astithas [:past] (away until 7/21) 2011-05-27 07:01:54 PDT
Created attachment 535626 [details] [diff] [review]
[in-devtools][checked-in] Simple fix

This simple fix returns the focus to the Scratchpad window, if still available. Handling the closed case, or displaying a view-source window with proper contents would involve some refactoring to keep the source code around.
Comment 3 Rob Campbell [:rc] (:robcee) 2011-05-27 12:32:16 PDT
Comment on attachment 535626 [details] [diff] [review]
[in-devtools][checked-in] Simple fix

simple enough for me.
Comment 4 :Gavin Sharp [email: gavin@gavinsharp.com] 2011-06-01 13:56:58 PDT
Comment on attachment 535626 [details] [diff] [review]
[in-devtools][checked-in] Simple fix

Where does this magic "Scratchpad" aSourceURL come from? I don't see any createMessageNode callers that would pass that in, but maybe this is on top of something other than mozilla-central?

Can there be multiple scratchpad windows, such that this results in us focusing the wrong one?
Comment 5 Panos Astithas [:past] (away until 7/21) 2011-06-02 04:50:15 PDT
(In reply to comment #4)
> Comment on attachment 535626 [details] [diff] [review] [review]
> Simple fix
> 
> Where does this magic "Scratchpad" aSourceURL come from? I don't see any
> createMessageNode callers that would pass that in, but maybe this is on top
> of something other than mozilla-central?

AFAIK it comes from the evalInSandbox() parameters, along with the line number:

http://mxr.mozilla.org/mozilla-central/source/browser/base/content/scratchpad.js#230

The patch is against the devtools repo, but there should be no differences from m-c.

> Can there be multiple scratchpad windows, such that this results in us
> focusing the wrong one?

I hadn't thought of that one, but this patch will focus on the most recent one, which should be sufficient most of the time.

The only case I can think of that wouldn't behave as expected is to have 2 scratchpads open, log from the first, switch to the second for some reason, then switch to the console and click the link. I don't think we have sufficient information to determine which window to focus at that point, though. Rob?
Comment 6 Kevin Dangoor 2011-06-02 05:58:39 PDT
Since the fix appears to be small and I'd rather not randomly send people to scratchpad.com, I'd like to get this into Aurora.
Comment 7 Rob Campbell [:rc] (:robcee) 2011-06-02 08:10:02 PDT
(In reply to comment #4)
> Comment on attachment 535626 [details] [diff] [review] [review]
> Simple fix
> 
> Where does this magic "Scratchpad" aSourceURL come from? I don't see any
> createMessageNode callers that would pass that in, but maybe this is on top
> of something other than mozilla-central?

http://mxr.mozilla.org/mozilla-central/source/browser/base/content/scratchpad.js#230

> Can there be multiple scratchpad windows, such that this results in us
> focusing the wrong one?

There can be. We should maybe make the Scratchpad "URI" correspond to a unique window. Maybe "Scratchpad/n:lineno"? Not sure if we want to do that in this bug or a future one.
Comment 8 Panos Astithas [:past] (away until 7/21) 2011-06-02 09:49:44 PDT
A new bug would increase the chances that this makes Aurora. I'd also try to get that one in, too, of course.
Comment 9 Rob Campbell [:rc] (:robcee) 2011-06-02 10:12:46 PDT
makes sense. Thanks!
Comment 10 Johnny Stenback (:jst, jst@mozilla.com) 2011-06-02 14:37:52 PDT
Not tracking this, but please request approval on a safe and reviewed patch once ready.
Comment 11 :Gavin Sharp [email: gavin@gavinsharp.com] 2011-06-02 15:33:50 PDT
Comment on attachment 535626 [details] [diff] [review]
[in-devtools][checked-in] Simple fix

let's get that followup filed
Comment 12 Panos Astithas [:past] (away until 7/21) 2011-06-02 23:28:00 PDT
Filed 661762 as a followup bug.
Comment 13 Dão Gottwald [:dao] 2011-06-08 04:51:58 PDT
Panos/Rob/Kevin: The [checkin-needed] whiteboard tag keeps confusing users (bug 311007 comment 42), as predicted in bug 577721 comment 43. Please come up with something different.
Comment 14 Panos Astithas [:past] (away until 7/21) 2011-06-08 06:02:14 PDT
I'm changing it to [can-land] after discussion with Kevin.
Comment 16 AndreiD[QA] 2011-06-16 05:04:37 PDT
The issue is not reproducible anymore on - if clicking on the link in the web console the scratchpad window is focused:
-> Windows 7:
Mozilla/5.0 (Windows NT 6.1; rv:7.0a1) Gecko/20110615 Firefox/7.0a1
-> Mac OS 10.6
Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:7.0a1) Gecko/20110615 Firefox/7.0a1
-> Windows XP
Mozilla/5.0 (Windows NT 5.1; rv:7.0a1) Gecko/20110615 Firefox/7.0a1
-> Ubuntu 11.04
Mozilla/5.0 (X11; Linux i686; rv:7.0a1) Gecko/20110615 Firefox/7.0a1

Setting status to VERIFIED

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