Last Comment Bug 751739 - Scratchpad could identify itself
: Scratchpad could identify itself
Status: RESOLVED FIXED
[good first bug][mentor=msucan][lang=...
:
Product: Firefox
Classification: Client Software
Component: Developer Tools: Scratchpad (show other bugs)
: 13 Branch
: All All
: P3 normal (vote)
: Firefox 18
Assigned To: Anton Kovalyov (:anton)
:
: Julian Descottes [:jdescottes]
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2012-05-03 15:47 PDT by Dave Camp (:dcamp)
Modified: 2012-09-13 06:24 PDT (History)
8 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Proposed fix (1.83 KB, patch)
2012-09-10 20:24 PDT, Anton Kovalyov (:anton)
no flags Details | Diff | Splinter Review
Add Scratchpad reference (via __SCRATCHPAD__) to both contentSandbox and chromeSandbox (1.98 KB, patch)
2012-09-11 13:23 PDT, Anton Kovalyov (:anton)
fayearthur: review+
Details | Diff | Splinter Review
Variation of the previous patch but with two additional asserts (2.24 KB, patch)
2012-09-11 14:18 PDT, Anton Kovalyov (:anton)
no flags Details | Diff | Splinter Review

Description Dave Camp (:dcamp) 2012-05-03 15:47:41 PDT
I've been playing with a restartless addon by loading its bootstrap.js in scratchpad.  I have one or two little things I need to do differently in the scratchpad vs. the addon.

I guess we're trying to avoid polluting the namespace with the scratchpad, but it would be cool if some sort of __SCRATCHPAD__ constant were defined when running in the scratchpad.
Comment 1 Cedric Vivier [:cedricv] 2012-05-03 23:00:35 PDT
Each Scratchpad window has a "Scratchpad" global variable that references the Scratchpad object instance for that window.
Comment 2 Dave Camp (:dcamp) 2012-05-04 08:58:49 PDT
But that's not actually available in the scratchpad's sandbox.  In a browser-context scratchpad you happen to have the browser.js Scratchpad object, but that's not really helpful.
Comment 3 Rob Campbell [:rc] (:robcee) 2012-08-02 09:39:55 PDT
do an mxr.mozilla.org search for scratchpad.js to find the source.
Comment 4 sam 2012-08-09 14:18:54 PDT
i'm looking to get involved with the mozilla community. anyone willing to help me get started on this bug, if not already assigned?
Comment 5 Amod [:AMoz] 2012-08-31 09:30:45 PDT
Sir, I would like to contribute to this bug. I would be highly obliged if u assign the bug to me...Will u explain the bug in detail ?
Comment 6 Mihai Sucan [:msucan] 2012-08-31 09:55:59 PDT
sam and Amod: thank you both for the interest in fixing this bug.

To fix this bug please take a look at browser/devtools/scratchpad. Read through scratchpad.js. There you can find how the browser context works and you can learn about how we use the Cu.Sandbox() object.

Please make a patch that adds a reference to the Scratchpad object (|this|) into the sandbox, so code we write can use it. See line 300 (the chromeSandbox getter). You can do this._chromeSandbox.Scratchpad = this;

Learn about Cu.Sandbox():
https://developer.mozilla.org/en-US/docs/Components.utils.Sandbox
https://developer.mozilla.org/en-US/docs/Components.utils.evalInSandbox

How to build Firefox:
https://developer.mozilla.org/En/Simple_Firefox_build

How to submit a patch:
https://developer.mozilla.org/En/Developer_Guide/How_to_Submit_a_Patch

Due to the situation we are in... the first one to provide a working patch will have the bug assigned to. Thanks!
Comment 7 Mihai Sucan [:msucan] 2012-08-31 14:05:04 PDT
To contributors (clarification):
by doing this._chromeSandbox.foobar = 'whatever'; you add new things into the sandbox scope - these become available when you execute any script in Scratchpad.

__SCRATCHPAD__ = this; makes the Scratchpad object available to any script that executes in Scratchpad.


Dave: to make sure we are on the same page here; do we want this new property only in browser context or both? Also, do we want to expose the Scratchpad object or you just want __SCRATCHPAD__ = true; ? I think it could be fancier to give access to the Scratchpad object (in browser context). Thanks!
Comment 8 Anton Kovalyov (:anton) 2012-09-10 20:24:40 PDT
Created attachment 659947 [details] [diff] [review]
Proposed fix

This is a pretty dumb fix for this feature request. It adds a reference to the Scratchpad object as a property on the contentSandbox named __SCRATCHPAD__. I added it to the contentSandbox because chromeSandbox is not available from within the Scratchpad window (or I am missing something).

Also, currently __SCRATCHPAD__ is available only if you load a file into Scratchpad window. Should it be available by default?

P.S. I hope I didn't screw up flags above.
Comment 9 Paul Rouget [:paul] 2012-09-11 03:10:59 PDT
Not sure to understand why you don't add it to the chrome sandbox as well. Did you see https://developer.mozilla.org/en/Tools/Scratchpad#Using_Scratchpad_to_access_Firefox_internals ?

About the patch format and the flags:
- you can't "screw up" too much. We can always fix things. So don't be afraid of trying things;
- if you think your patch is ready for review, no need to ask for feedback;
- no need to do that for now, but for future patches, try to include the header to your patch (`hg export tip` does that for example).
Comment 10 Anton Kovalyov (:anton) 2012-09-11 12:42:37 PDT
Thanks, Paul. I didn't see that document, I'll update the patch.
Comment 11 Anton Kovalyov (:anton) 2012-09-11 13:23:29 PDT
Created attachment 660207 [details] [diff] [review]
Add Scratchpad reference (via __SCRATCHPAD__) to both contentSandbox and chromeSandbox

This patch adds __SCRATCHPAD__ property to both contentSandbox and chromeSandbox. The change is covered by two tests that simply check for the __SCRATCHPAD__ existence (do I need to check objects further?).
Comment 12 Mihai Sucan [:msucan] 2012-09-11 13:26:16 PDT
(In reply to anton from comment #11)
> Created attachment 660207 [details] [diff] [review]
> Add Scratchpad reference (via __SCRATCHPAD__) to both contentSandbox and
> chromeSandbox
> 
> This patch adds __SCRATCHPAD__ property to both contentSandbox and
> chromeSandbox. The change is covered by two tests that simply check for the
> __SCRATCHPAD__ existence (do I need to check objects further?).

Anton, patch looks good. Thank you!

You might want to check for a property on the __SCRATCHPAD__ object, but nothing fancy.
Comment 13 Heather Arthur [:harth] 2012-09-11 13:51:34 PDT
Comment on attachment 660207 [details] [diff] [review]
Add Scratchpad reference (via __SCRATCHPAD__) to both contentSandbox and chromeSandbox

Awesome, thanks! Mihai's suggestion is good, we could check for a property of the scratchpad object in the test, maybe __SCRATCHPAD__.editor?
Comment 14 Anton Kovalyov (:anton) 2012-09-11 14:18:14 PDT
Created attachment 660220 [details] [diff] [review]
Variation of the previous patch but with two additional asserts

Added two additional asserts that check for __SCRATCHPAD__.editor existence.
Comment 15 Heather Arthur [:harth] 2012-09-11 15:40:04 PDT
http://hg.mozilla.org/integration/fx-team/rev/c1ed30f63bcb
Comment 16 Victor Porof [:vporof][:vp] 2012-09-13 03:46:14 PDT
https://hg.mozilla.org/mozilla-central/rev/c1ed30f63bcb

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