Francesco Lodolo [:flod] Reporter
	Description • 11 years ago

https://hg.mozilla.org/mozilla-central/rev/920669704476#l7.12

<!ENTITY updateAuto.label "Automatically install updates (recommended: improved security)">
<!ENTITY updateAuto.label "Automatically install updates from desktop &brandShortName;">

Considering how the string has changed, please change the name of this entity.

	Brian R. Bondy [:bbondy] Assignee
	Comment 1 • 11 years ago

Is this string change approved from UX?

	u60234
	Comment 2 • 11 years ago

Another problem with this string is that is shown not only on Windows 8, but on other OS's as well where it doesn't make any sense at all.

	Brian R. Bondy [:bbondy] Assignee
	Comment 3 • 11 years ago

We could fix that by dynamically changing the label, but I like the more simple solution of just changing the string as per Comment 0. Just waiting for Yuan to confirm this is OK.

	Brian R. Bondy [:bbondy] Assignee
	Comment 4 • 11 years ago

Sorry ignore Comment 3, I misinterpreted what Comment 0 meant. 
I think comment 0 is just for the entity name itself and not the string right?
On pre-win8 machines I think we need to dynamically change the string to what it was before instead of including "from Desktop Firefox"

	Francesco Lodolo [:flod] Reporter
	Comment 5 • 11 years ago

(In reply to Brian R. Bondy [:bbondy] from comment #4)
> Sorry ignore Comment 3, I misinterpreted what Comment 0 meant. 

https://developer.mozilla.org/en-US/docs/Making_String_Changes

Once landed on mozilla-central, if you want to change a string you have to change its entity name.

	Brian R. Bondy [:bbondy] Assignee
	Comment 6 • 11 years ago

Understood, thanks for the link!

	Brian R. Bondy [:bbondy] Assignee
	Comment 7 • 11 years ago

Questions for Yuan and rstrong before I do this work to avoid going back and forth.  I'd like to do this before the next migration if there is work to be done:

- Do we need to update pre -win8 to change back to: "Automatically install updates (recommended: improved security)" from what it currently is: "Automatically install updates from desktop &brandShortName;"
- (rstrong only) Do we need to update the entity label? The link quote above says you don't need to unless you are changing the meaning, I think the meaning stays the same, even know there is a slight modification to it to not say recommended.
- Do we want the (recommended: improved security) note re-added in win8?  If so do we add it to the metro and desktop option?

	Francesco Lodolo [:flod] Reporter
	Comment 8 • 11 years ago

> - (rstrong only) Do we need to update the entity label? The link quote above
> says you don't need to unless you are changing the meaning, I think the
> meaning stays the same, even know there is a slight modification to it to
> not say recommended.

I'm not rstrong, but the problem here is not the "general meaning", you introduced a variable there and changing the entity name is the only way to force localizers to notice that change.

	Robert Strong (they/them - no direct email)
	Comment 9 • 11 years ago

(In reply to Brian R. Bondy [:bbondy] from comment #7)
> Questions for Yuan and rstrong before I do this work to avoid going back and
> forth.  I'd like to do this before the next migration if there is work to be
> done:
> 
> - Do we need to update pre -win8 to change back to: "Automatically install
> updates (recommended: improved security)" from what it currently is:
> "Automatically install updates from desktop &brandShortName;"
I think that makes sense but UX needs to make that call.

> - (rstrong only) Do we need to update the entity label? The link quote above
> says you don't need to unless you are changing the meaning, I think the
> meaning stays the same, even know there is a slight modification to it to
> not say recommended.
Since there are localizers that localize nightly please change the entity name to avoid confusion.

> - Do we want the (recommended: improved security) note re-added in win8?  If
> so do we add it to the metro and desktop option?
If it is in any of the messages I think that it should be in all of them but that is a UX call.

	Yuan Wang(:Yuan) – Mobile Firefox Design Lead
	Comment 10 • 11 years ago

1. Do we need to update pre -win8 to change back to: "Automatically install updates (recommended: improved security)" from what it currently is:"Automatically install updates from desktop &brandShortName;"? 

For pre Windows 8 updates, that makes sense to me. And on Mac, it has the same message. 

2. Do we want the (recommended: improved security) note re-added in win8?  If so do we add it to the metro and desktop option?

I think we shouldn't add the note back in Windows 8. We have given "Not checking updates" a security risk flag. I think there is enough information to inform users about security risks. 

Adding to "Update desktop" and "Update metro and desktop" doesn't really help the users make a decision. And there is already a note about add-on compatibility. Adding one more is going to cause information overload...

Adding to either one of these options may create confusion. Users should make their decision based on the add-on compatibility not security risks. We should focus on showing the most important note, in order to help users make their choice.

	Brian R. Bondy [:bbondy] Assignee
	Comment 11 • 11 years ago

I'd like to try to get this landed on m-c before the uplift if possible.

	Robert Strong (they/them - no direct email)
	Comment 12 • 11 years ago

Comment on attachment 764175 [details] [diff] [review]
Patch v1.

Discussed a couple of problems with bbondy on irc and he will be resubmitting

	Brian R. Bondy [:bbondy] Assignee
	Comment 13 • 11 years ago

- Had mistakenly added the dynamic string to the dtd instead of the properties file. 
- When I previously tested, I was mistakenly looking at the first string instead of the 2nd in metro builds (which was correct for in metro).

	Robert Strong (they/them - no direct email)
	Comment 14 • 11 years ago

Comment on attachment 764376 [details] [diff] [review]
Patch v2.

>diff --git a/browser/locales/en-US/chrome/browser/preferences/advanced.dtd b/browser/locales/en-US/chrome/browser/preferences/advanced.dtd
>--- a/browser/locales/en-US/chrome/browser/preferences/advanced.dtd
>+++ b/browser/locales/en-US/chrome/browser/preferences/advanced.dtd
>@@ -79,24 +79,25 @@
> <!ENTITY clearOfflineAppCacheNow.label   "Clear Now">
> <!ENTITY clearOfflineAppCacheNow.accesskey "N">
> <!ENTITY overrideSmartCacheSize.label    "Override automatic cache management">
> <!ENTITY overrideSmartCacheSize.accesskey "O">
> 
> <!ENTITY updateTab.label                 "Update">
> 
> <!ENTITY updateApp.label                 "&brandShortName; updates:">
>-<!ENTITY updateAuto.label                "Automatically install updates from desktop &brandShortName;">
>-<!ENTITY updateAuto.accesskey            "A">
> <!ENTITY updateAutoMetro.label           "Automatically update from desktop and Windows 8 style &brandShortName;">
> <!ENTITY updateAutoMetro.accesskey       "s">
> <!ENTITY updateCheck.label               "Check for updates, but let me choose whether to install them">
> <!ENTITY updateCheck.accesskey           "C">
> <!ENTITY updateManual.label              "Never check for updates (not recommended: security risk)">
> <!ENTITY updateManual.accesskey          "N">
>+<!-- Note either updateAutoNoMetro is used or (updateAutoMetro and updateAutoDesktop), so re-using accesss key in updateAutoNoMetro is OK -->
Mention that updateAutoDesktop is in preferences.properties
What about the accesskey for updateAutoDesktop?

>+<!ENTITY updateAutoNoMetro.label         "Automatically install updates (recommended: improved security)">
>+<!ENTITY updateAutoNoMetro.accesskey     "A">
Please position this where updateAuto was removed.
Please rename to updateAuto1 or updateAutoDefault

Removing r? until the accesskey question is answered

	Robert Strong (they/them - no direct email)
	Comment 15 • 11 years ago

btw: I usually go with a separate label, keep all of the strings in the dtd, and just hide the label(s) as appropriate... it just seems cleaner code and string wise.

	Brian R. Bondy [:bbondy] Assignee
	Comment 16 • 11 years ago

(In reply to Robert Strong [:rstrong] (do not email) from comment #15)
> btw: I usually go with a separate label, keep all of the strings in the dtd,
> and just hide the label(s) as appropriate... it just seems cleaner code and
> string wise.

Normally I do this for labels, and I did this for the previous bug, but in this case there is code that checks the radio button value that would have to be more complicated, so I think in this case it's better to use the properties file instead.

	Robert Strong (they/them - no direct email)
	Comment 18 • 11 years ago

Comment on attachment 764515 [details] [diff] [review]
Patch v3.

(In reply to Robert Strong [:rstrong] (do not email) from comment #14)
<snip>
> >+<!ENTITY updateAutoNoMetro.label         "Automatically install updates (recommended: improved security)">
> >+<!ENTITY updateAutoNoMetro.accesskey     "A">
> Please position this where updateAuto was removed.
> Please rename to updateAuto1 or updateAutoDefault
You didn't rename updateAutoNoMetro

r=me with that

	Brian R. Bondy [:bbondy] Assignee
	Comment 19 • 11 years ago

Implemented missed nit.  Carrying forward r+.

	Brian R. Bondy [:bbondy] Assignee
	Comment 20 • 11 years ago

https://hg.mozilla.org/integration/mozilla-inbound/rev/83aa31ec53d9

	Ed Morley [:emorley]
	Comment 21 • 11 years ago

https://hg.mozilla.org/mozilla-central/rev/83aa31ec53d9

	Mihaela Velimiroviciu (:mihaelav)
	Comment 22 • 11 years ago

updateAuto.label string was modified in bug 866229 - https://hg.mozilla.org/mozilla-central/rev/920669704476:
-<!ENTITY updateAuto.label                "Automatically install updates (recommended: improved security)">
+<!ENTITY updateAuto.label                "Automatically install updates from desktop &brandShortName;">
 <!ENTITY updateAuto.accesskey            "A">

Verified the string is updated in the latest Nightly:
Mozilla/5.0 (Windows NT 6.2; rv:25.0) Gecko/20130710 Firefox/25.0, build id: 20130710030205