Note: There are a few cases of duplicates in user autocompletion which are being worked on.

Scratchpad should provide a quick way to reload the tab and re-run the code

RESOLVED FIXED in Firefox 18

Status

()

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

People

(Reporter: cedricv, Assigned: anton)

Tracking

Trunk
Firefox 18
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [good first bug][mentor=past][lang=js])

Attachments

(1 attachment, 2 obsolete attachments)

(Reporter)

Description

5 years ago
I propose the following addition to the Execute menu (Run is Ctrl-R):

Reload tab and run  Ctrl-Shift-R


This could help when doing 'intrusive' prototyping with the Scratchpad (eg. doing massive DOM changes).

Thoughts?
it could be useful, though we'd have to disable that in Browser context. Not sure it warrants its own menu item or not.
(Reporter)

Comment 2

5 years ago
(In reply to Rob Campbell [:rc] (:robcee) from comment #1)
> it could be useful, though we'd have to disable that in Browser context. Not
> sure it warrants its own menu item or not.

FWIW, I don't think it deserves a place in the context menu (actually I have a bit of a mixed opinion with our current use of the context menu with duplication of most of Execute).
But the Execute menu otoh is far from feeling crowded imho.

Kevin, what do you think?

Comment 3

5 years ago
I agree that this seems reasonable for the Execute menu.

One thing to consider here is that when we pull the tools together into a combined or separate window interface (and if that includes Scratchpad in some form), users will want to be able to reload the page from within the tools. That might change our thoughts around keyboard shortcuts here.
We still want this. Good first bug?

Filter on BLACKEAGLE?!
Priority: -- → P3
Whiteboard: [good first bug][mentor=past][lang=js]

Comment 5

5 years ago
i'm looking to make my first contribution and saw this bug as a great opportunity. anyone willing to help me out?
(In reply to sam from comment #5)
> i'm looking to make my first contribution and saw this bug as a great
> opportunity. anyone willing to help me out?

Awesome!

First, Panos said he'd volunteer to mentor for this bug, so I don't want to step on his toes, but I can give you a few pointers to get started.

Source code is located in http://mxr.mozilla.org/mozilla-central/source/browser/devtools/scratchpad/.

Pay particular attention to scratchpad.js therein.

Check out: https://developer.mozilla.org/en-US/docs/Developer_Guide, notably the https://developer.mozilla.org/En/Developer_Guide/Source_Code and https://developer.mozilla.org/En/Developer_Guide/Build_Instructions.

If I haven't scared you off yet, join us in irc://irc.mozilla.org/#devtools and say hi. We're happy to help you get setup and hacking.
(In reply to Rob Campbell [:rc] (:robcee) from comment #6)
> (In reply to sam from comment #5)
> > i'm looking to make my first contribution and saw this bug as a great
> > opportunity. anyone willing to help me out?
> 
> Awesome!
> 
> First, Panos said he'd volunteer to mentor for this bug, so I don't want to
> step on his toes, but I can give you a few pointers to get started.
> 
> Source code is located in
> http://mxr.mozilla.org/mozilla-central/source/browser/devtools/scratchpad/.
> 
> Pay particular attention to scratchpad.js therein.
> 
> Check out: https://developer.mozilla.org/en-US/docs/Developer_Guide, notably
> the https://developer.mozilla.org/En/Developer_Guide/Source_Code and
> https://developer.mozilla.org/En/Developer_Guide/Build_Instructions.
> 
> If I haven't scared you off yet, join us in irc://irc.mozilla.org/#devtools
> and say hi. We're happy to help you get setup and hacking.

Yes, as Rob says feel free to ask for help if you encounter any obstacles you can't overcome on your own.

Comment 8

5 years ago
Sir, I would like to contribute to this bug. If the bug is not assigned to anybody then I would be highly obliged if u assign the bug to me...Will u explain the bug in detail ?
(Assignee)

Comment 9

5 years ago
Created attachment 660507 [details] [diff] [review]
Proposed patch (no tests yet).

Attached is a proposed (working) patch. It adds a new menu item "Reload And Run" that is enabled only within the content context. Clicking on it will reload the page and run the entire editor as soon as Window object is ready.

(Initially I planned to use the 'load' event but then figured that this way we will prevent people from executing their Scratchpad code before the page is fully loaded. And if they want to wait for that they can always use window.addEventListener('load', ...); themselves)

I'd like to ask for feedback while I'm working on a patch:

 1. Is the overall approach reasonable? 
 2. This is the first time I'm modifying XUL and other non-JS files. Does everything look good there?

Thanks!
Assignee: nobody → anton
Status: NEW → ASSIGNED
Attachment #660507 - Flags: feedback?(fayearthur)
Attachment #660507 - Flags: feedback?(dcamp)
(In reply to Anton Kovalyov (:anton) from comment #9)
> Created attachment 660507 [details] [diff] [review]
> Proposed patch (no tests yet).
> 
> Attached is a proposed (working) patch. It adds a new menu item "Reload And
> Run" that is enabled only within the content context. Clicking on it will
> reload the page and run the entire editor as soon as Window object is ready.
> 
> (Initially I planned to use the 'load' event but then figured that this way
> we will prevent people from executing their Scratchpad code before the page
> is fully loaded. And if they want to wait for that they can always use
> window.addEventListener('load', ...); themselves)

The first time you write a scratchpad or open one you wouldn't want an window onload handler. Once you've switched to reloading and running, you'd have to add an onload handler. It seems like you'd have to switch between these, and adding+removing an onload handler or using boilerplate wouldn't be ideal, imo.

>  2. This is the first time I'm modifying XUL and other non-JS files. Does
> everything look good there?

Looks good to me!

It seems like this could use a keyboard shortcut. Otherwise you could reload the page, switch back to scratchpad and re-run the scratchpad in about as much time using the keyboard.
(Assignee)

Comment 11

5 years ago
Created attachment 660583 [details] [diff] [review]
Proposed patch with tests.

Attached is my patch ready for review. Per Heather's comment we decided to wait for the 'load' event instead of DOMWindowCreated to avoid confusion. Also added a shortcut for Reload And Run (command-E).
Attachment #660507 - Attachment is obsolete: true
Attachment #660507 - Flags: feedback?(fayearthur)
Attachment #660507 - Flags: feedback?(dcamp)
Attachment #660583 - Flags: review?(fayearthur)
Comment on attachment 660583 [details] [diff] [review]
Proposed patch with tests.

Review of attachment 660583 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good, nice test, thanks!

Just a couple nits and removing of event listeners, otherwise good to go.

::: browser/devtools/scratchpad/scratchpad.js
@@ +451,4 @@
>    },
>  
>    /**
> +   * Reload the current page and execute the entire editor content when

nit: break up lines that go over 80 chars.

::: browser/devtools/scratchpad/test/browser_scratchpad_bug740948_reload_and_run.js
@@ +13,5 @@
> +{
> +  waitForExplicitFinish();
> +  Services.prefs.setBoolPref(DEVTOOLS_CHROME_ENABLED, true);
> +
> +	gBrowser.selectedTab = gBrowser.addTab();

Looks like there are some tabs in this test, two-space indentation for js.

@@ +51,5 @@
> +  sp.setText(EDITOR_TEXT);
> +
> +  let browser = gBrowser.selectedBrowser;
> +
> +  browser.addEventListener("DOMWindowCreated", function () {

Remove this event listener after you catch the event.

@@ +52,5 @@
> +
> +  let browser = gBrowser.selectedBrowser;
> +
> +  browser.addEventListener("DOMWindowCreated", function () {
> +    browser.contentWindow.addEventListener("foo", function () {

Remove this even listener too after catching the event.
Attachment #660583 - Flags: review?(fayearthur) → review+
(Assignee)

Comment 13

5 years ago
Created attachment 661076 [details] [diff] [review]
Final patch
Attachment #660583 - Attachment is obsolete: true
(Assignee)

Comment 14

5 years ago
Browser Chrome Test Summary
	Passed: 6888
	Failed: 0
	Todo: 9

*** End BrowserChrome Test Results ***
Whiteboard: [good first bug][mentor=past][lang=js] → [good first bug][mentor=past][lang=js][land-in-fx-team]
As discussed in IRC, we're going with the original shortcut key that was mentioned in the bug, Ctrl-Shift-R.

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