Closed
Bug 1367959
Opened 7 years ago
Closed 7 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•7 years ago
|
Attachment #8871583 -
Flags: review?(jaws)
Assignee | ||
Comment 2•7 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•7 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•7 years ago
|
||
Thanks for reminding, Francesco. I've update the patch to sync the old andnew prefs.
Updated•7 years ago
|
Whiteboard: [photon-preference][triage] → [photon-preference]
Comment 6•7 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•7 years ago
|
||
Thanks for reviewing, Jared. Let's land the patch.
Keywords: checkin-needed
Comment 8•7 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•7 years ago
|
Keywords: checkin-needed
Comment hidden (mozreview-request) |
Comment 11•7 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•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/84e9dbe2e9e3
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
•