Evaluating console.log or other API messages in Scratchpad presents wrong source link in web console

VERIFIED FIXED in Firefox 6

Status

()

Firefox
Developer Tools
VERIFIED FIXED
6 years ago
6 years ago

People

(Reporter: rc, Assigned: past)

Tracking

unspecified
Firefox 6
Points:
---

Firefox Tracking Flags

(firefox6- fixed)

Details

(Whiteboard: [scratchpad][console][view-source][fixed-in-devtools][merged-to-mozilla-central])

Attachments

(1 attachment)

(Reporter)

Description

6 years ago
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.
Assignee: nobody → past
Status: NEW → ASSIGNED

Comment 1

6 years ago
If the patch for this is not risky-looking, we should nominate this for Firefox 6.
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.
Attachment #535626 - Flags: review?(rcampbell)
(Reporter)

Comment 3

6 years ago
Comment on attachment 535626 [details] [diff] [review]
[in-devtools][checked-in] Simple fix

simple enough for me.
Attachment #535626 - Flags: review?(rcampbell) → review+
Attachment #535626 - Flags: review?(gavin.sharp)
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?
(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

6 years ago
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.
tracking-firefox6: --- → ?
(Reporter)

Comment 7

6 years ago
(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.
A new bug would increase the chances that this makes Aurora. I'd also try to get that one in, too, of course.
(Reporter)

Comment 9

6 years ago
makes sense. Thanks!
Not tracking this, but please request approval on a safe and reviewed patch once ready.
tracking-firefox6: ? → -
Comment on attachment 535626 [details] [diff] [review]
[in-devtools][checked-in] Simple fix

let's get that followup filed
Attachment #535626 - Flags: review?(gavin.sharp) → review+
Attachment #535626 - Flags: approval-mozilla-aurora+
Filed 661762 as a followup bug.
Whiteboard: [scratchpad][console][view-source] → [scratchpad][console][view-source][checkin-needed]
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.
I'm changing it to [can-land] after discussion with Kevin.
Whiteboard: [scratchpad][console][view-source][checkin-needed] → [scratchpad][console][view-source][can-land]
Whiteboard: [scratchpad][console][view-source][can-land] → [scratchpad][console][view-source][fixed-in-devtools]
Attachment #535626 - Attachment description: Simple fix → [in-devtools] Simple fix

Updated

6 years ago
Whiteboard: [scratchpad][console][view-source][fixed-in-devtools] → [scratchpad][console][view-source][fixed-in-devtools][merged-to-mozilla-central]

Updated

6 years ago
Attachment #535626 - Attachment description: [in-devtools] Simple fix → [in-devtools][checked-in] Simple fix

Comment 15

6 years ago
http://hg.mozilla.org/mozilla-central/rev/588eea08e466
http://hg.mozilla.org/releases/mozilla-aurora/rev/f54affede577
status-firefox6: --- → fixed
Target Milestone: --- → Firefox 6

Updated

6 years ago
Status: ASSIGNED → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED

Comment 16

6 years ago
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
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.