Closed
Bug 1367959
Opened 8 years ago
Closed 8 years ago
Remove brandShortName from the string "… can improve &brandShortName; performance"
Categories
(Firefox :: Settings UI, enhancement, P1)
Tracking
()
VERIFIED
FIXED
Firefox 55
People
(Reporter: evanxd, Assigned: evanxd)
References
Details
(Whiteboard: [photon-preference])
Attachments
(2 files)
Base Michelle's recommendation from [1], remove brandShortName from the string "… can improve &brandShortName; performance". And this is the spec[1].
[1]: https://bugzilla.mozilla.org/show_bug.cgi?id=1364070#c17
[2]: https://mozilla.invisionapp.com/share/C2B97CAYH#/screens/230367021
Flags: qe-verify+
Comment hidden (mozreview-request) |
Assignee | ||
Updated•8 years ago
|
Attachment #8871583 -
Flags: review?(jaws)
Assignee | ||
Comment 2•8 years ago
|
||
Hi Jared,
Could you help review the patch?
Michelle suggest us to update the string at https://bugzilla.mozilla.org/show_bug.cgi?id=1364070#c17.
Thanks.
Comment 3•8 years ago
|
||
mozreview-review |
Comment on attachment 8871583 [details]
Bug 1367959 - Remove brandShortName from the string "… can improve &brandShortName; performance".
https://reviewboard.mozilla.org/r/143066/#review146770
::: browser/locales/en-US/chrome/browser/preferences-old/advanced.dtd:134
(Diff revision 1)
> "Use recommended performance settings">
> <!ENTITY useRecommendedPerformanceSettings.description
> "These settings are tailored to your computer’s hardware and operating system.">
> <!ENTITY useRecommendedPerformanceSettings.accesskey
> "U">
> -<!ENTITY limitContentProcess.label "Content process limit">
> +<!ENTITY limitContentProcess2.label "Content process limit">
I've noticed this in the previous patch: if the string doesn't change, you shouldn't change the ID just to match the others, it creates unnecessary churn for localizers. The only exception is if you change a .label, then the .accesskey should match.
But… since this has already become a big mess with old and new prefs (they're completely out of sync), I would rename them to something more useful, e.g. limitContentProcessOption.(label|description|accesskey) and sync both old and new prefs.
And, for the future, avoid them going out of sync like this.
Comment hidden (mozreview-request) |
Assignee | ||
Comment 5•8 years ago
|
||
Thanks for reminding, Francesco. I've update the patch to sync the old andnew prefs.
Updated•8 years ago
|
Whiteboard: [photon-preference][triage] → [photon-preference]
Comment 6•8 years ago
|
||
mozreview-review |
Comment on attachment 8871583 [details]
Bug 1367959 - Remove brandShortName from the string "… can improve &brandShortName; performance".
https://reviewboard.mozilla.org/r/143066/#review146958
Attachment #8871583 -
Flags: review?(jaws) → review+
Assignee | ||
Comment 7•8 years ago
|
||
Thanks for reviewing, Jared. Let's land the patch.
Keywords: checkin-needed
Comment 8•8 years ago
|
||
We're sorry, Autoland could not rebase your commits for you automatically. Please manually rebase your commits and try again.
hg error in cmd: hg rebase -s 81ec88533854 -d 3a1143c269e5: rebasing 398492:81ec88533854 "Bug 1367959 - Remove brandShortName from the string "… can improve &brandShortName; performance". r=jaws" (tip)
merging browser/components/preferences/in-content-old/main.xul
merging browser/locales/en-US/chrome/browser/preferences-old/advanced.dtd
warning: conflicts while merging browser/locales/en-US/chrome/browser/preferences-old/advanced.dtd! (edit, then use 'hg resolve --mark')
unresolved conflicts (see hg resolve, then hg rebase --continue)
Updated•8 years ago
|
Keywords: checkin-needed
Comment hidden (mozreview-request) |
Comment 11•8 years ago
|
||
Pushed by philringnalda@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/84e9dbe2e9e3
Remove brandShortName from the string "… can improve &brandShortName; performance". r=jaws
Keywords: checkin-needed
Comment 12•8 years ago
|
||
bugherder |
Comment 13•7 years ago
|
||
Hi Evan,
Can you please provide some steps(if any) or additional information, in order for QA to verify this issue?
Thank you.
Flags: needinfo?(evan)
Assignee | ||
Comment 14•7 years ago
|
||
Hi Deac,
The bug is about a string change. You could just verify the string change in the red box of the attachment screenshot. And this is the spec[1] for the string change.
Thank you.
[1]: https://mozilla.invisionapp.com/share/C2B97CAYH#/screens/230367021
Flags: needinfo?(evan)
Comment 15•7 years ago
|
||
Build ID: 20170810100255
User Agent Mozilla/5.0 (Macintosh; Intel Mac OS X 10.12; rv:57.0) Gecko/20100101 Firefox/57.0
Verified as fixed on Firefox Nightly 57.0a1 on Windows 10 x 64, Mac OS X 10.12 and Ubuntu 16.04 x64.
You need to log in
before you can comment on or make changes to this bug.
Description
•