Closed
Bug 882598
Opened 11 years ago
Closed 11 years ago
Defect - Rename updateAuto.label to reflect string change
Categories
(Firefox for Metro Graveyard :: Install/Update, defect, P1)
Firefox for Metro Graveyard
Install/Update
Tracking
(Not tracked)
VERIFIED
FIXED
Firefox 24
People
(Reporter: flod, Assigned: bbondy)
References
Details
(Whiteboard: feature=defect c=Settings_pane_options_and_about u=metro_firefox_user p=1)
Attachments
(1 file, 3 obsolete files)
(deleted),
patch
|
bbondy
:
review+
|
Details | Diff | Splinter Review |
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.
Updated•11 years ago
|
Blocks: metrov1triage
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.
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.
Assignee | ||
Comment 4•11 years ago
|
||
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.
Assignee | ||
Comment 6•11 years ago
|
||
Understood, thanks for the link!
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?
Flags: needinfo?(robert.bugzilla)
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.
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.
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.
Flags: needinfo?(ywang)
Assignee | ||
Comment 11•11 years ago
|
||
I'd like to try to get this landed on m-c before the uplift if possible.
Assignee: nobody → netzen
Status: NEW → ASSIGNED
Attachment #764175 -
Flags: review?(robert.bugzilla)
Assignee | ||
Updated•11 years ago
|
No longer depends on: 866229
Whiteboard: feature=defect c=Settings_pane_options_and_about u=metro_firefox_user p=1
Assignee | ||
Updated•11 years ago
|
Summary: Rename updateAuto.label to reflect string change → Defect - Rename updateAuto.label to reflect string change
Assignee | ||
Updated•11 years ago
|
Priority: -- → P1
Updated•11 years ago
|
Flags: needinfo?(robert.bugzilla)
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
Attachment #764175 -
Flags: review?(robert.bugzilla)
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).
Attachment #764376 -
Flags: review?(robert.bugzilla)
Assignee | ||
Updated•11 years ago
|
Attachment #764175 -
Attachment is obsolete: true
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
Attachment #764376 -
Flags: review?(robert.bugzilla)
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.
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.
Assignee | ||
Comment 17•11 years ago
|
||
Attachment #764376 -
Attachment is obsolete: true
Attachment #764515 -
Flags: review?(robert.bugzilla)
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
Attachment #764515 -
Flags: review?(robert.bugzilla) → review+
Assignee | ||
Comment 19•11 years ago
|
||
Implemented missed nit. Carrying forward r+.
Attachment #764515 -
Attachment is obsolete: true
Attachment #764532 -
Flags: review+
Assignee | ||
Comment 20•11 years ago
|
||
Target Milestone: --- → Firefox 24
Comment 21•11 years ago
|
||
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
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
Status: RESOLVED → VERIFIED
You need to log in
before you can comment on or make changes to this bug.
Description
•