Closed Bug 657136 Opened 9 years ago Closed 9 years ago

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

Categories

(DevTools :: General, defect)

defect
Not set

Tracking

(Not tracked)

VERIFIED FIXED
Firefox 6

People

(Reporter: rcampbell, Assigned: rcampbell)

References

Details

(Keywords: l12y, ux-jargon, Whiteboard: [scratchpad][fixed-in-devtools][merged-to-mozilla-central])

Attachments

(1 file, 4 obsolete files)

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", ...
bug 657172 could resolve this bug.
Assignee: nobody → rcampbell
Status: NEW → ASSIGNED
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.
Attached patch rename context to environment (obsolete) — Splinter Review
rename context menu to environment, renamed chrome to browser. tests pass.
Attachment #533316 - Flags: review?(mihai.sucan)
Blocks: 657131
Attached patch rename context to environment (obsolete) — Splinter Review
fixed conflicting accesskey
Attachment #533316 - Attachment is obsolete: true
Attachment #533316 - Flags: review?(mihai.sucan)
Attachment #533324 - Flags: review?(mihai.sucan)
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.
Attachment #533324 - Flags: review?(mihai.sucan) → review+
(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).
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...
Attached patch rename context to environment (obsolete) — Splinter Review
renamed method to setBrowserContext
Attachment #533324 - Attachment is obsolete: true
Attachment #533582 - Flags: review?(sdwilsh)
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...)
yeah, well-spotted. I saw that after attaching the previous version. Will update, thanks!
updated with corrected constant.
Attachment #533582 - Attachment is obsolete: true
Attachment #533582 - Flags: review?(sdwilsh)
Attachment #533711 - Flags: review?(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)
Attachment #533711 - Flags: review?(sdwilsh)
Whiteboard: [scratchpad] → [scratchpad][fixed-in-devtools]
Comment on attachment 533711 [details] [diff] [review]
[checked-in] [in-devtools] rename context to environment 2

http://hg.mozilla.org/projects/devtools/rev/ac4c8689e717
Attachment #533711 - Attachment description: rename context to environment 2 → [in-devtools] rename context to environment 2
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Whiteboard: [scratchpad][fixed-in-devtools] → [scratchpad][fixed-in-devtools][merged-to-mozilla-central]
Target Milestone: --- → Firefox 6
Comment on attachment 533711 [details] [diff] [review]
[checked-in] [in-devtools] rename context to environment 2

http://hg.mozilla.org/mozilla-central/rev/ac4c8689e717
Attachment #533711 - Attachment description: [in-devtools] rename context to environment 2 → [checked-in] [in-devtools] rename context to environment 2
Why were the old strings commented out in scratchpad.dtd? It's less confusing to a localizer if obsolete strings are removed.
We can do that.
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!
Attachment #533711 - Attachment is obsolete: true
Attachment #534292 - Flags: review?(sdwilsh)
reopening for comment cleanup.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Attachment #534292 - Flags: review?(sdwilsh) → review+
http://hg.mozilla.org/mozilla-central/rev/0ad1aae1c559
Status: REOPENED → RESOLVED
Closed: 9 years ago9 years ago
Resolution: --- → FIXED
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.
Status: RESOLVED → VERIFIED
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.