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

VERIFIED FIXED in Firefox 6

Status

()

Firefox
Developer Tools
VERIFIED FIXED
6 years ago
6 years ago

People

(Reporter: rc, Assigned: rc)

Tracking

({l12y, ux-jargon})

unspecified
Firefox 6
l12y, ux-jargon
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [scratchpad][fixed-in-devtools][merged-to-mozilla-central])

Attachments

(1 attachment, 4 obsolete attachments)

(Assignee)

Description

6 years ago
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", ...
(Assignee)

Comment 1

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

Comment 2

6 years ago
bug 657172 could resolve this bug.
(Assignee)

Updated

6 years ago
Assignee: nobody → rcampbell
Status: NEW → ASSIGNED
(Assignee)

Comment 3

6 years ago
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.
(Assignee)

Comment 4

6 years ago
Created attachment 533316 [details] [diff] [review]
rename context to environment

rename context menu to environment, renamed chrome to browser. tests pass.
Attachment #533316 - Flags: review?(mihai.sucan)
(Assignee)

Updated

6 years ago
Blocks: 657131
(Assignee)

Comment 5

6 years ago
Created attachment 533324 [details] [diff] [review]
rename context to environment

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).
(Assignee)

Comment 8

6 years ago
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...
(Assignee)

Comment 9

6 years ago
Created attachment 533582 [details] [diff] [review]
rename context to environment

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...)
(Assignee)

Comment 11

6 years ago
yeah, well-spotted. I saw that after attaching the previous version. Will update, thanks!
(Assignee)

Comment 12

6 years ago
Created attachment 533711 [details] [diff] [review]
[checked-in] [in-devtools] rename context to environment 2

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

Updated

6 years ago
Whiteboard: [scratchpad] → [scratchpad][fixed-in-devtools]
(Assignee)

Comment 14

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

Updated

6 years ago
Status: ASSIGNED → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
Whiteboard: [scratchpad][fixed-in-devtools] → [scratchpad][fixed-in-devtools][merged-to-mozilla-central]
Target Milestone: --- → Firefox 6

Comment 15

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

Comment 16

6 years ago
Why were the old strings commented out in scratchpad.dtd? It's less confusing to a localizer if obsolete strings are removed.
(Assignee)

Comment 17

6 years ago
We can do that.
(Assignee)

Comment 18

6 years ago
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!
Attachment #533711 - Attachment is obsolete: true
Attachment #534292 - Flags: review?(sdwilsh)
(Assignee)

Comment 19

6 years ago
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
Last Resolved: 6 years ago6 years ago
Resolution: --- → FIXED

Comment 21

6 years ago
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.

Updated

6 years ago
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.