Scratchpad could identify itself

RESOLVED FIXED in Firefox 18

Status

()

Firefox
Developer Tools: Scratchpad
P3
normal
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: dcamp, Assigned: anton)

Tracking

13 Branch
Firefox 18
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [good first bug][mentor=msucan][lang=js][fixed-in-fx-team])

Attachments

(1 attachment, 2 obsolete attachments)

(Reporter)

Description

5 years ago
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.
Each Scratchpad window has a "Scratchpad" global variable that references the Scratchpad object instance for that window.
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → WORKSFORME
(Reporter)

Updated

5 years ago
Resolution: WORKSFORME → INVALID
(Reporter)

Comment 2

5 years ago
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.
Status: RESOLVED → REOPENED
Resolution: INVALID → ---
do an mxr.mozilla.org search for scratchpad.js to find the source.
Priority: -- → P3
Whiteboard: [good first bug][mentor=msucan][lang=js]

Comment 4

5 years ago
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

5 years ago
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 ?
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!
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!
(Assignee)

Comment 8

5 years ago
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.
Attachment #659947 - Flags: review?(fayearthur)
Attachment #659947 - Flags: feedback?(fayearthur)

Comment 9

5 years ago
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).
Assignee: nobody → anton
Status: REOPENED → ASSIGNED
(Assignee)

Comment 10

5 years ago
Thanks, Paul. I didn't see that document, I'll update the patch.
(Assignee)

Comment 11

5 years ago
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?).
Attachment #659947 - Attachment is obsolete: true
Attachment #659947 - Flags: review?(fayearthur)
Attachment #659947 - Flags: feedback?(fayearthur)
Attachment #660207 - Flags: review?(dylanhelling)
(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.
(Assignee)

Updated

5 years ago
Attachment #660207 - Flags: review?(dylanhelling) → review?(fayearthur)
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?
Attachment #660207 - Flags: review?(fayearthur) → review+
(Assignee)

Comment 14

5 years ago
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.
Attachment #660207 - Attachment is obsolete: true
(Assignee)

Updated

5 years ago
Whiteboard: [good first bug][mentor=msucan][lang=js] → [good first bug][mentor=msucan][lang=js][land-in-fx-team]
http://hg.mozilla.org/integration/fx-team/rev/c1ed30f63bcb
Whiteboard: [good first bug][mentor=msucan][lang=js][land-in-fx-team] → [good first bug][mentor=msucan][lang=js][fixed-in-fx-team]
https://hg.mozilla.org/mozilla-central/rev/c1ed30f63bcb
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago5 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 18
You need to log in before you can comment on or make changes to this bug.