Last Comment Bug 657136 - Rename top-level Context menu to something less confusing in Scratchpad
: Rename top-level Context menu to something less confusing in Scratchpad
Status: VERIFIED FIXED
[scratchpad][fixed-in-devtools][merge...
: l12y, ux-jargon
Product: Firefox
Classification: Client Software
Component: Developer Tools (show other bugs)
: unspecified
: All All
: -- normal (vote)
: Firefox 6
Assigned To: Rob Campbell [:rc] (:robcee)
:
Mentors:
Depends on:
Blocks: 657131
  Show dependency treegraph
 
Reported: 2011-05-14 08:19 PDT by Rob Campbell [:rc] (:robcee)
Modified: 2011-05-26 07:16 PDT (History)
8 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
rename context to environment (13.11 KB, patch)
2011-05-18 10:08 PDT, Rob Campbell [:rc] (:robcee)
no flags Details | Diff | Review
rename context to environment (13.11 KB, patch)
2011-05-18 10:51 PDT, Rob Campbell [:rc] (:robcee)
mihai.sucan: review+
Details | Diff | Review
rename context to environment (13.70 KB, patch)
2011-05-19 03:11 PDT, Rob Campbell [:rc] (:robcee)
no flags Details | Diff | Review
[checked-in] [in-devtools] rename context to environment 2 (15.42 KB, patch)
2011-05-19 11:09 PDT, Rob Campbell [:rc] (:robcee)
no flags Details | Diff | Review
scratchpad comment removal (1.83 KB, patch)
2011-05-22 06:01 PDT, Rob Campbell [:rc] (:robcee)
gavin.sharp: review+
Details | Diff | Review

Description Rob Campbell [:rc] (:robcee) 2011-05-14 08:19:54 PDT
The current top-level menu name "Context" in the Scratchpad tool is too confusing. Since "Context" is usually used to refer to right-click menus, we'd prefer to change the top-level menu name to something else.

The menu is currently used to change which execution "context" to evaluate JavaScript against: content or chrome. Alternative suggestions for this have been, "Environment", "Sandbox", "Playground", "Bounds", "Enclosure", ...
Comment 1 Rob Campbell [:rc] (:robcee) 2011-05-14 08:27:14 PDT
Created discussion at: https://groups.google.com/forum/#!topic/mozilla.dev.l10n/ohHOMW8hTOo
Comment 2 Erik Vold 2011-05-14 15:51:19 PDT
bug 657172 could resolve this bug.
Comment 3 Rob Campbell [:rc] (:robcee) 2011-05-18 07:13:05 PDT
Looks like we're going with Environment based on feedback in the discussion group.

Also suggested was a change from "Chrome" context (or, Environment as it will be known) to something more localizable. I've suggested "Browser" though "Application" was another candidate.
Comment 4 Rob Campbell [:rc] (:robcee) 2011-05-18 10:08:34 PDT
Created attachment 533316 [details] [diff] [review]
rename context to environment

rename context menu to environment, renamed chrome to browser. tests pass.
Comment 5 Rob Campbell [:rc] (:robcee) 2011-05-18 10:51:11 PDT
Created attachment 533324 [details] [diff] [review]
rename context to environment

fixed conflicting accesskey
Comment 6 Mihai Sucan [:msucan] 2011-05-18 11:30:51 PDT
Comment on attachment 533324 [details] [diff] [review]
rename context to environment

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

I am giving the patch a tentative r+, but I find it not-so-nice that we have a bit of "frankenstein"/hybrid code. We still have API that refers to the chrome context, instead of the browser context, even variable names, and you also left the old strings in the scratchpad.dtd. I am not sure if this is a reason to "punt" on the patch, to give it r-. Please ask a browser peer.

Otherwise the patch is fine.
Comment 7 Shawn Wilsher :sdwilsh 2011-05-18 11:40:20 PDT
(In reply to comment #6)
> I am giving the patch a tentative r+, but I find it not-so-nice that we have a
> bit of "frankenstein"/hybrid code. We still have API that refers to the chrome
> context, instead of the browser context, even variable names, and you also left
> the old strings in the scratchpad.dtd. I am not sure if this is a reason to
> "punt" on the patch, to give it r-. Please ask a browser peer.
I would prefer to see chrome context be renamed to browser context as well.  Otherwise, it just gets complicated to think about (and it already kinda is).
Comment 8 Rob Campbell [:rc] (:robcee) 2011-05-19 02:18:16 PDT
Shawn, so you're +1 in favor of changing the method names as well? It sounds like you are, so I'll go ahead and make those changes. I was trying to restrict the modifications to strings and ids alone.

standby for a review request...
Comment 9 Rob Campbell [:rc] (:robcee) 2011-05-19 03:11:15 PDT
Created attachment 533582 [details] [diff] [review]
rename context to environment

renamed method to setBrowserContext
Comment 10 Mihai Sucan [:msucan] 2011-05-19 07:51:37 PDT
Perhaps we should also rename the executionContext constant SCRATCHPAD_CONTEXT_CHROME to SCRATCHPAD_CONTEXT_BROWSER, evalInChromeSandbox() to evalInBrowserSandbox(), and chromeSandbox to browserSandbox.

(I know, ugly, but if we do it, we should do it entirely...)
Comment 11 Rob Campbell [:rc] (:robcee) 2011-05-19 10:58:57 PDT
yeah, well-spotted. I saw that after attaching the previous version. Will update, thanks!
Comment 12 Rob Campbell [:rc] (:robcee) 2011-05-19 11:09:37 PDT
Created attachment 533711 [details] [diff] [review]
[checked-in] [in-devtools] rename context to environment 2

updated with corrected constant.
Comment 13 Shawn Wilsher :sdwilsh 2011-05-19 13:55:45 PDT
Comment on attachment 533711 [details] [diff] [review]
[checked-in] [in-devtools] rename context to environment 2

I am sufficiently satisfied with Mihai's review here.  I don't need to look at this (rs=sdwilsh)
Comment 14 Rob Campbell [:rc] (:robcee) 2011-05-19 14:13:04 PDT
Comment on attachment 533711 [details] [diff] [review]
[checked-in] [in-devtools] rename context to environment 2

http://hg.mozilla.org/projects/devtools/rev/ac4c8689e717
Comment 15 Dave Camp (:dcamp) 2011-05-21 21:27:25 PDT
Comment on attachment 533711 [details] [diff] [review]
[checked-in] [in-devtools] rename context to environment 2

http://hg.mozilla.org/mozilla-central/rev/ac4c8689e717
Comment 16 Hasse 2011-05-22 03:09:12 PDT
Why were the old strings commented out in scratchpad.dtd? It's less confusing to a localizer if obsolete strings are removed.
Comment 17 Rob Campbell [:rc] (:robcee) 2011-05-22 05:34:16 PDT
We can do that.
Comment 18 Rob Campbell [:rc] (:robcee) 2011-05-22 06:01:52 PDT
Created attachment 534292 [details] [diff] [review]
scratchpad comment removal

not sure who to tag for review on this. It should land this weekend onto mozilla-central for tuesday's merge. If anyone spots it, please r+ and land, thanks!
Comment 19 Rob Campbell [:rc] (:robcee) 2011-05-22 06:02:12 PDT
reopening for comment cleanup.
Comment 20 :Gavin Sharp [email: gavin@gavinsharp.com] 2011-05-23 11:03:33 PDT
http://hg.mozilla.org/mozilla-central/rev/0ad1aae1c559
Comment 21 AndreiD[QA] 2011-05-26 06:48:07 PDT
Verified fixed on:
Windows 7:
Mozilla/5.0 (Windows NT 6.1; rv:6.0a2) Gecko/20110525 Firefox/6.0a2
Window XP:
Mozilla/5.0 (Windows NT 5.1; rv:6.0a2) Gecko/20110525 Firefox/6.0a2
Mac OS 10.6
Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:6.0a2) Gecko/20110525 Firefox/6.0a2 
Linux i686:
Mozilla/5.0 (X11; Linux i686; rv:6.0a2) Gecko/20110525 Firefox/6.0a2

*Note: There is a top menu "Environment" option in Scratchpad. Marking this as Verified.

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