Closed Bug 841686 Opened 8 years ago Closed 8 years ago

Work - Update strings, order, and options for image context menu

Categories

(Firefox for Metro Graveyard :: General, defect)

x86_64
Windows 8.1
defect
Not set
normal

Tracking

(Not tracked)

VERIFIED FIXED

People

(Reporter: jimm, Assigned: jimm)

References

Details

Attachments

(2 files)

We're changing the story to ask for this: 

    Save to Picture Library
    Copy Image
    Copy Image location
    Open Image in New Tab

So, what we need are for the items to be re-arranged to put Save to Picture Library first, followed by Copy Image, Copy Image Location, and Open Image in New Tab. We need to get rid of Save image to.
Assignee: nobody → jmathies
Attached patch patchSplinter Review
Attachment #714306 - Flags: review?(mbrubeck)
There's a minor bug in this, the url of the image is in mediaURL, but the copy link function copies linkURL. Which is why we take the time to write tests. :) I'll update the final when I land it.
Comment on attachment 714306 [details] [diff] [review]
patch

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

::: browser/metro/base/content/browser.xul
@@ +588,1 @@
>            </richlistitem>

just a reminder to fix this (as you noted in comment 2)

::: browser/metro/locales/en-US/chrome/browser.dtd
@@ +99,5 @@
> +<!-- l10n: contextSaveImageLib saves an image to the users "Pictures Library"
> +     (Explorer -> left hand side navigation ppane -> Libraries -> Pictures) -->
> +<!ENTITY contextSaveImageLib.label        "Save to Picture Library">
> +<!ENTITY contextCopyImage.label           "Copy Image">
> +<!ENTITY contextCopyImageLocation.label   "Copy Image location">

capital L for Location
Attachment #714306 - Flags: review?(mbrubeck) → review+
(In reply to Matt Brubeck (:mbrubeck) from comment #3)
> ::: browser/metro/locales/en-US/chrome/browser.dtd
> @@ +99,5 @@
> > +<!-- l10n: contextSaveImageLib saves an image to the users "Pictures Library"
> > +     (Explorer -> left hand side navigation ppane -> Libraries -> Pictures) -->
> > +<!ENTITY contextSaveImageLib.label        "Save to Picture Library">
> > +<!ENTITY contextCopyImage.label           "Copy Image">
> > +<!ENTITY contextCopyImageLocation.label   "Copy Image location">
> 
> capital L for Location

The text in the Story doesn't have that in it. Is there some sort of convention we adhere to or should we copy the text in the write up verbatim?
(In reply to Jim Mathies [:jimm] from comment #4)
> > > +<!ENTITY contextSaveImageLib.label        "Save to Picture Library">
> > > +<!ENTITY contextCopyImage.label           "Copy Image">
> > > +<!ENTITY contextCopyImageLocation.label   "Copy Image location">
> > 
> > capital L for Location
> 
> The text in the Story doesn't have that in it. Is there some sort of
> convention we adhere to or should we copy the text in the write up verbatim?

We should consistently use either sentence case or title case.  "Copy Image location" is neither, so I assume it was a typo in the story.

Microsoft's Windows Store App Guidelines For Typography say "stick with sentence or title case for content":
http://msdn.microsoft.com/en-us/library/windows/apps/jj553415.aspx

...and the guidelines for context menus specifically say "Use sentence capitalization for each command name":
http://msdn.microsoft.com/en-us/library/windows/apps/hh465308.aspx

...which suggests that most of our command labels need to be changed.
Flags: needinfo?(asa)
For the reference, this is the proposal I came up with with a bit help of testpilot data. It was reviewed + by Madhava, Firefox UX lead. 

The Image contextual actions listed in Comment 1 look right to me. This proposal also include actions for other context menus, such as "Image links", "Selected text links" as well.
(In reply to Yuan Wang(:Yuan) – Firefox UX Team from comment #6)
> Created attachment 714566 [details]
> Context menu design proposal
> 
> For the reference, this is the proposal I came up with with a bit help of
> testpilot data. It was reviewed + by Madhava, Firefox UX lead. 
> 
> The Image contextual actions listed in Comment 1 look right to me. This
> proposal also include actions for other context menus, such as "Image
> links", "Selected text links" as well.

This is excellent, thanks yuan. I will update the image context accordingly.

I'll also post this on the metro-contextmenus tracker for reference.
Flags: needinfo?(asa)
https://hg.mozilla.org/mozilla-central/rev/8b45c32b6028
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Verified that the Image context menu items are correct. Testing build from http://hg.mozilla.org/mozilla-central/rev/702d2814efbf
Status: RESOLVED → VERIFIED
Summary: Update strings, order, and options for image context menu → Work - Update strings, order, and options for image context menu
OS: Windows 8 Metro → Windows 8.1
You need to log in before you can comment on or make changes to this bug.