751739 - Scratchpad could identify itself

Status: RESOLVED FIXED

(DevTools Graveyard :: Scratchpad, defect, P3)

Description

[Dave Camp (:dcamp)]

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]]

Each Scratchpad window has a "Scratchpad" global variable that references the Scratchpad object instance for that window.

Comment 2

[Dave Camp (:dcamp)]

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)]

do an mxr.mozilla.org search for scratchpad.js to find the source.

Comment 4

[sam]

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]]

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]]

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]]

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)]

Attachment: 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]]

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)]

Thanks, Paul. I didn't see that document, I'll update the patch.

Comment 11

[Anton Kovalyov (:anton)]

Attachment: 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]]

(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]]

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)]

Attachment: 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]]

http://hg.mozilla.org/integration/fx-team/rev/c1ed30f63bcb

Comment 16

[Victor Porof [:vporof][:vp]]

https://hg.mozilla.org/mozilla-central/rev/c1ed30f63bcb