657136 - Rename top-level Context menu to something less confusing in Scratchpad

Status: VERIFIED FIXED

(DevTools :: General, defect)

Description

[Rob Campbell [:rc] (:robcee)]

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

Created discussion at: https://groups.google.com/forum/#!topic/mozilla.dev.l10n/ohHOMW8hTOo

Comment 2

[Erik Vold]

bug 657172 could resolve this bug.

Comment 3

[Rob Campbell [:rc] (:robcee)]

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

Attachment: rename context to environment

rename context menu to environment, renamed chrome to browser. tests pass.

Comment 5

[Rob Campbell [:rc] (:robcee)]

Attachment: rename context to environment

fixed conflicting accesskey

Comment 6

[Mihai Sucan [:msucan]]

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]

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

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

Attachment: rename context to environment

renamed method to setBrowserContext

Comment 10

[Mihai Sucan [:msucan]]

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

yeah, well-spotted. I saw that after attaching the previous version. Will update, thanks!

Comment 12

[Rob Campbell [:rc] (:robcee)]

Attachment: [checked-in] [in-devtools] rename context to environment 2

updated with corrected constant.

Comment 13

[Shawn Wilsher :sdwilsh]

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

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

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

[u60234]

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

We can do that.

Comment 18

[Rob Campbell [:rc] (:robcee)]

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

reopening for comment cleanup.

Comment 20

[:Gavin Sharp [email: [e-mail]]]

http://hg.mozilla.org/mozilla-central/rev/0ad1aae1c559

Comment 21

[AndreiD[QA]]

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.