Closed Bug 1355788 Opened 3 years ago Closed 2 years ago

rename "Appearance" to "Themes"

Categories

(Toolkit :: Add-ons Manager, enhancement, P4)

enhancement

Tracking

()

RESOLVED FIXED
mozilla57
Tracking Status
firefox57 --- fixed

People

(Reporter: designakt, Assigned: andy+bugzilla)

References

(Blocks 1 open bug)

Details

(Whiteboard: triaged)

Attachments

(1 file)

The add-ons managers left hand side menu is the only place that uses the word "Appearance" when referring to "Themes", everywhere else we use the word "Themes". Therefor we should also use "Themes" in the add-ons manager menu to be consistent.

Jorge has confirmed this in 
https://bugzilla.mozilla.org/show_bug.cgi?id=1254301#c12
Priority: -- → P4
Whiteboard: triaged
Blocks: 1387450
Assignee: nobody → amckay
Attachment #8900752 - Flags: review?(aswan) → review?(francesco.lodolo)
Comment on attachment 8900752 [details]
bug 1355788 rename Appearance to Themes

https://reviewboard.mozilla.org/r/172208/#review177458

::: toolkit/locales/en-US/chrome/mozapps/extensions/extensions.properties:185
(Diff revision 2)
>  
>  #LOCALIZATION NOTE (eulaHeader) %S is name of the add-on asking the user to agree to the EULA
>  eulaHeader=%S requires that you accept the following End User License Agreement before installation can proceed:
>  
>  type.extension.name=Extensions
> -type.theme.name=Appearance
> +type.theme.name=Themes

Sorry, you need a new ID for this type of changes
https://developer.mozilla.org/en-US/docs/Mozilla/Localization/Localization_content_best_practices#Changing_existing_strings
Attachment #8900752 - Flags: review?(francesco.lodolo) → review-
(In reply to Francesco Lodolo [:flod] from comment #3)
> Sorry, you need a new ID for this type of changes

And the ids that we use here are programmatically generated :(
(In reply to Andrew Swan [:aswan] from comment #4)
> (In reply to Francesco Lodolo [:flod] from comment #3)
> > Sorry, you need a new ID for this type of changes
> 
> And the ids that we use here are programmatically generated :(

Can you point me to the code? It would be strange if there's no way to map it to a different ID.

By changing the string like this, there's basically no way to ensure that the content will be ever updated. Localizers won't even be aware of the change, because the string is already translated in their file.
There was at least one other place that depended upon the value of aID being theme. It seemed to just make all this simpler if I just remove the dynamically generated IDs. Something that :flod said was bad anyways :)
Comment on attachment 8900752 [details]
bug 1355788 rename Appearance to Themes

https://reviewboard.mozilla.org/r/172208/#review177452

I assume you tested this by hand?
Also, there's (at least?) one more provider that you didn't change in `browser/modules/Experiments.jsm`
Attachment #8900752 - Flags: review?(aswan) → review+
Comment on attachment 8900752 [details]
bug 1355788 rename Appearance to Themes

https://reviewboard.mozilla.org/r/172208/#review177452

I did, I ran a few tests even. Thanks for spotting Experiments.jsm, I've added that in.
Comment on attachment 8900752 [details]
bug 1355788 rename Appearance to Themes

https://reviewboard.mozilla.org/r/172208/#review177458

> Sorry, you need a new ID for this type of changes
> https://developer.mozilla.org/en-US/docs/Mozilla/Localization/Localization_content_best_practices#Changing_existing_strings

Changed from type.theme to type.themes
Keywords: checkin-needed
Pushed by ryanvm@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/03f4526ce1e7
rename Appearance to Themes r=aswan
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/03f4526ce1e7
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Duplicate of this bug: 1254301
You need to log in before you can comment on or make changes to this bug.