1381066 - about:preferences - strings to support cloud storage download

Status: RESOLVED WONTFIX

(Firefox :: Settings UI, enhancement, P3)

Description

[Punam Dahiya [:pdahiya]]

Implement preferences content UI to handle browser.download.folderList value 3 which indicates default download location is elsewhere as specified by
cloud storage API getDownloadFolder

Under Preferences -> Files & Applications -> Downloads

Downloads section should show third radio button with option
‘Save files to <Preferred Cloud storage Provider> Download folder' if cloud.services.storage.key preference key is set.

Comment 1

[Punam Dahiya [:pdahiya]]

With back-end Cloud Storage module (Bug 1357171) landed in nightly. System add-on bug 1365129 has this last dependency on updating about:preference UI with download preference set using cloud storage API (pref browser.download.folderList 3)

We show the additional third radio button if the system add-on is present, or if the user has the right prefs set.

Third radio button can have label ‘Save file to ‘ <provider name>. It's visible if pref browser.download.folderlist is 3, pref cloud.services.storage.key exists with provider key as value and browser.download.useDownloadDir is true.

It's is hidden if above conditions are not met.

We can use 'Save files to' from
https://dxr.mozilla.org/mozilla-central/source/browser/locales/en-US/chrome/browser/preferences/main.dtd

NI Ryan for UX inputs.

Ryan, we briefly discussed this in SF. Should we localize providers name for shield studies? If yes, we need localized string for ‘Dropbox’ and ‘GDrive’ in https://dxr.mozilla.org/mozilla-central/source/browser/locales/en-US/chrome/browser/preferences/preferences.properties.

Thanks!

Comment 2

[Punam Dahiya [:pdahiya]]

and my comment got truncated..

Gijs reminded that if we need localized strings for preferences UI for 56, it should land before 56 merges to beta coming Wednesday. Please provide your inputs. Thanks!

Comment 3

[:Gijs (he/him)]

Note that reuse of the string is likely not OK because in some locales the string would need to change depending on context / where people are saving files (ie "dropbox" might be a different gender in some languages than the file picker).

I would suggest creating a new string in a .properties file that has a replacement variable (%S) in which we put the names of the cloud provider (which I believe should themselves also be localized as separate strings in the .properties file).

Comment 4

[Punam Dahiya [:pdahiya]]

Attachment: Bug 1381066 - Cloud Storage API - Preferences Content UI strings

* Preferences Content UI Cloud Storage strings

Review commit: https://reviewboard.mozilla.org/r/163102/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/163102/

Comment 5

[Punam Dahiya [:pdahiya]]

This is the first patch with strings that will be used by the code fix to display additional radio button with label ‘Save files to <cloud storage provider name>’.

Marking Ryan for content and ui-review. Please review. Thanks!

Link to try server
https://treeherder.mozilla.org/#/jobs?repo=try&revision=b789d80ca6b3b214b04861f834462fbf18b8835a

Comment 6

[:Gijs (he/him)]

Comment on attachment 8892099 [details]
Bug 1381066 - Cloud Storage API - Preferences Content UI strings

https://reviewboard.mozilla.org/r/163102/#review168500

codewise, this is just an rs, but the strings need approval from the UI/UX folks to land.

Comment 7

[:Gijs (he/him)]

Radar'ing this to flod.

Comment 8

[Francesco Lodolo [:flod]]

Comment on attachment 8892099 [details]
Bug 1381066 - Cloud Storage API - Preferences Content UI strings

https://reviewboard.mozilla.org/r/163102/#review168616

::: browser/locales/en-US/chrome/browser/preferences/preferences.properties:81
(Diff revision 1)
>  downloadsFolderName=Downloads
>  chooseDownloadFolderTitle=Choose Download Folder:
>  
> +#### Cloud Storage Downloads
> +
> +saveFilesToCloudStorage=Save files to %S

When there's a placeholder, always add a localization comment explaining what the replacement will be.

Comment 9

[Punam Dahiya [:pdahiya]]

Comment on attachment 8892099 [details]
Bug 1381066 - Cloud Storage API - Preferences Content UI strings

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/163102/diff/1-2/

Comment 10

[Punam Dahiya [:pdahiya]]

(In reply to Francesco Lodolo [:flod] from comment #8)
> Comment on attachment 8892099 [details]
> Bug 1381066 - Cloud Storage API - Preferences Content UI strings
> 
> https://reviewboard.mozilla.org/r/163102/#review168616
> 
> :::
> browser/locales/en-US/chrome/browser/preferences/preferences.properties:81
> (Diff revision 1)
> >  downloadsFolderName=Downloads
> >  chooseDownloadFolderTitle=Choose Download Folder:
> >  
> > +#### Cloud Storage Downloads
> > +
> > +saveFilesToCloudStorage=Save files to %S
> 
> When there's a placeholder, always add a localization comment explaining
> what the replacement will be.
Thanks flod, Updated patch with the feedback.

Comment 11

[Francesco Lodolo [:flod]]

Comment on attachment 8892099 [details]
Bug 1381066 - Cloud Storage API - Preferences Content UI strings

https://reviewboard.mozilla.org/r/163102/#review168842

::: browser/locales/en-US/chrome/browser/preferences/preferences.properties:81
(Diff revision 2)
>  downloadsFolderName=Downloads
>  chooseDownloadFolderTitle=Choose Download Folder:
>  
> +#### Cloud Storage Downloads
> +
> +# LOCALIZATION NOTE (saveFilesToCloudStorage): %S = Folder name e.g. Dropbox, Google Drive

Maybe "Folder name" should be "Service name"?

For example: 

# LOCALIZATION NOTE (saveFilesToCloudStorage): %S is replaced by a service name (Dropbox, Google Drive)

Comment 12

[Punam Dahiya [:pdahiya]]

Comment on attachment 8892099 [details]
Bug 1381066 - Cloud Storage API - Preferences Content UI strings

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/163102/diff/2-3/

Comment 13

[Punam Dahiya [:pdahiya]]

Comment on attachment 8892099 [details]
Bug 1381066 - Cloud Storage API - Preferences Content UI strings

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/163102/diff/3-4/

Comment 14

[Punam Dahiya [:pdahiya]]

(In reply to Francesco Lodolo [:flod] from comment #11)
> > +#### Cloud Storage Downloads
> > +
> > +# LOCALIZATION NOTE (saveFilesToCloudStorage): %S = Folder name e.g. Dropbox, Google Drive
> 
> Maybe "Folder name" should be "Service name"?
> 
> For example: 
> 
> # LOCALIZATION NOTE (saveFilesToCloudStorage): %S is replaced by a service
> name (Dropbox, Google Drive)

Updated patch with feedback. Removed strings dropboxFolderName, gdriveFolderName. As per discussion with mconnor and mkaply, service name should be the display name from provider.json. Service name stays same across region and need not be translated. If we need to support localized names that should be in the json, because it's likely not in most regions.

Comment 15

[Francesco Lodolo [:flod]]

(In reply to Punam Dahiya [:pdahiya] from comment #14)
> Updated patch with feedback. Removed strings dropboxFolderName,
> gdriveFolderName. As per discussion with mconnor and mkaply, service name
> should be the display name from provider.json. Service name stays same
> across region and need not be translated. If we need to support localized
> names that should be in the json, because it's likely not in most regions.

I'm not sure that's the right approach, at least without a discussion.

I checked this morning out of curiosity the Arabic version of Google Drive, and the Chinese version of Dropbox. They both keep the brand unchanged (easy to spot)
https://www.google.com/intl/ar/drive/

But then, if you dig a little deeper, you'll notice that Google Drive is translated in Polish, and adapted to the grammatical case (check the last blurb and button, Dysku)
https://www.google.com/intl/pl/drive/

Dropbox has branding guidelines, I haven't been able to find any for Google Drive, besides a trademark for "Google Drive™ online storage service"
https://www.dropbox.com/branding
https://www.google.com/permissions/trademark/trademark-list.html

Note that you might ask them to not translate or transliterate the brand, but you still need to adapt the name to the grammatical case, for example by adding a desinence.

Comment 16

[Mike Kaply [:mkaply]]

I think we would still not want these in a translation file because it would be up to the localizer to figure out that they need to translate the same as Google and/or dropbox does (and we're not sure they use the same translation for the name of the directory on disk). Otherwise they would simple translate "Google Drive" to whatever they thought it should be.

To do this properly, we need to explicitly find out the name of Google Drive in all these cases and how it used.

Comment 17

[Punam Dahiya [:pdahiya]]

Looking at the about:preferences implementation, for an additional radio button, we need an access key that should be translated.

https://dxr.mozilla.org/mozilla-central/source/browser/components/preferences/in-content-new/main.xul#648

a) Should I request string saveFilesToCloudStorage.accessKey in preferences.properties or it should be done in dtd (https://dxr.mozilla.org/mozilla-central/source/browser/locales/en-US/chrome/browser/preferences/main.dtd)

b) For about:preferences, are access keys determined by certain rules or it can be an unused letter?

NI Gijs, Jaws to help answer above queries. Thanks!

Comment 18

[:Gijs (he/him)]

(In reply to Punam Dahiya [:pdahiya] from comment #17)
> Looking at the about:preferences implementation, for an additional radio
> button, we need an access key that should be translated.
> 
> https://dxr.mozilla.org/mozilla-central/source/browser/components/
> preferences/in-content-new/main.xul#648
> 
> a) Should I request string saveFilesToCloudStorage.accessKey in
> preferences.properties or it should be done in dtd
> (https://dxr.mozilla.org/mozilla-central/source/browser/locales/en-US/chrome/
> browser/preferences/main.dtd)

It should be in the same place as where you define the label string.

> b) For about:preferences, are access keys determined by certain rules or it
> can be an unused letter?

Uh, ideally it should be a letter that occurs in the label string. Better if it's at the start of a unique-ish word, which there isn't really in this string because it's more or less the same as the other "save files to" item.

I think we could just omit the access key here, certainly for the add-on case for 56.

Comment 19

[Jared Wein [:jaws] (please needinfo? me)]

Michelle needs to review the language here and Cindy/Tina should review if this preference should exist as well as where. Flagging them for needinfo...

Comment 20

[Ryan Feeley [:rfeeley]]

We should internationalize the provider strings for sure, but i’m not sure if the brand names themselves are actually localized. Mconnor imagines that translating product names like Dropbox would not be required.

Comment 21

[Punam Dahiya [:pdahiya]]

Thanks Jaws for helping identify respective owners.

Hi Cindy, Tina, Michelle

We need this string to label a third radio button under Preferences -> Files & Applications -> Downloads, that's shown if the system add-on consuming cloud storage API (Bug 1365129) is present, or if the user has the right prefs set (See Comment 1 and 2).  Please feel free to NI, IRC or vidyo for any questions. 

Thanks!

Comment 22

[Ryan Feeley [:rfeeley]]

I definitely recommend attaching a screenshot for us non-Firefox-building UX folks.

Comment 23

[:Gijs (he/him)]

(In reply to Ryan Feeley [:rfeeley] from comment #22)
> I definitely recommend attaching a screenshot for us non-Firefox-building UX
> folks.

There isn't any UI yet, this is purely about getting the strings right and landed before 56 merges to beta. Given that merge day is TOMORROW, it would be great to have approval/reviews/agreement ASAP so we can still get the strings into 56 without having to then barter away drinks, chocolate, etc. with l10n and relman folks to get it uplifted to 56 beta. :-)

Comment 24

[Francesco Lodolo [:flod]]

Per side discussion with mconnor, it looks like this is an en-US only experiment for 56, so a possibility is not expose strings for localization at all in 56. It's not ideal, but that would mean more flexibility and less need to rush.

Comment 25

[Punam Dahiya [:pdahiya]]

Attachment: Preferences UI with cloud storage pref set

Attaching about:preferences UI generated with WIP patch handling cloud storage preferences

Comment 26

[:Gijs (he/him)]

(In reply to Francesco Lodolo [:flod] from comment #24)
> Per side discussion with mconnor, it looks like this is an en-US only
> experiment for 56, so a possibility is not expose strings for localization
> at all in 56. It's not ideal, but that would mean more flexibility and less
> need to rush.

OK, so in that case and given the 56 ship has sailed now, should we wontfix this?

Comment 27

[Ryan Feeley [:rfeeley]]

My 2¢. It's a little odd to put the "always ask" option in the middle of the other two options. I'd put it at the top of the list if possible (as I believe that's the browser default option) or at the bottom if that's not possible for the experiment.

Comment 28

[Ryan Feeley [:rfeeley]]

Comment on attachment 8892099 [details]
Bug 1381066 - Cloud Storage API - Preferences Content UI strings

String looks good to me

Comment 29

[Punam Dahiya [:pdahiya]]

(In reply to :Gijs from comment #26)
> (In reply to Francesco Lodolo [:flod] from comment #24)
> > Per side discussion with mconnor, it looks like this is an en-US only
> > experiment for 56, so a possibility is not expose strings for localization
> > at all in 56. It's not ideal, but that would mean more flexibility and less
> > need to rush.
> 
> OK, so in that case and given the 56 ship has sailed now, should we wontfix
> this?

Yes, string PR is aligned with 57 and won’t fix for 56.  Aiming for about:preferences UI fix landed and uplifted in 56 for an en-US experiment.

Keeping this bug to track strings only for 57, and will open a separate bug for UI fix. Thanks!

Comment 30

[Punam Dahiya [:pdahiya]]

(In reply to Ryan Feeley [:rfeeley] from comment #27)
> My 2¢. It's a little odd to put the "always ask" option in the middle of the
> other two options. I'd put it at the top of the list if possible (as I
> believe that's the browser default option) or at the bottom if that's not
> possible for the experiment.

Thanks Ryan for review! Save files to <system default download directory> is the browser default option (browser.download.useDownloadDir as true and browser.download.folderList value as 1).
I can try to move "Always ask" option to the bottom so that it doesn't show in the middle when cloud storage pref is shown.

Comment 31

[Punam Dahiya [:pdahiya]]

Created bug 1387153 to implement preferences UI fix.

Comment 32

[Tina Hsieh[:Tina_Hsieh] UX]

Hi,
Thanks Ryan. The new order of the options makes more sense to me :) 
Regarding to "Dropbox", we add an app/service icon in front of the 3rd-party app/service name in Project Jazz. I wonder if we can align with that?

Comment 33

[:Gijs (he/him)]

We decided not to do this and instead implemented in bug 1387153 with hardcoded strings.