624405 - Localize Theme Descriptions (was: Upload Finnish descriptions of themes to Mercurial)

Status: RESOLVED FIXED

(SeaMonkey :: Preferences, enhancement)

Description

[Aki Laaksovirta]

User-Agent:       Mozilla/5.0 (X11; U; Linux i686 (x86_64); fi; rv:1.9.1.16) Gecko/20101123 Lightning/1.0b1 SeaMonkey/2.0.11
Build Identifier: 

So, I have translated the descriptions of both "SeaMonkey Default" and SeaMonkey Modern" themes, and I would like to see someone import it to Mercurial.

Reproducible: Always

Comment 1

[Aki Laaksovirta]

Attachment: The Finnish description strings

Comment 2

[Aki Laaksovirta]

Comment on attachment 502511 [details] [diff] [review]
The Finnish description strings

>diff --git a/suite/themes/classic/install.rdf b/suite/themes/classic/install.rdf
>--- a/suite/themes/classic/install.rdf
>+++ b/suite/themes/classic/install.rdf
>@@ -33,12 +33,23 @@
>     <em:appManaged>true</em:appManaged>
> 
>     <em:locked>true</em:locked>
> 
>     <!-- Front End Integration Hooks (used by Theme Manager)-->
>     <em:creator>Mozilla Contributors</em:creator>
>     <em:contributor>Mozilla Contributors</em:contributor>
> 
>+    <em:localized>
>+      <Description>
>+        <em:locale>fi</em:locale>
>+        <em:name>Oletus</em:name>
>+        <em:description>Tämä teema hyödyntää järjestelmän tarjoamia tyylejä sekä värejä sopiakseen hyvin yhteen muiden sovellusten kanssa.</em:description>
>+        <em:creator>Mozilla-projektin tukijat</em:creator>
>+        <em:contributor>Mozilla-projektin tukijat</em:contributors>
>+        <em:translator>Aki Laaksovirta</em:translator>
>+      </Description>
>+    </em:localized>
>+
>     <em:internalName>classic/1.0</em:internalName>
>   </Description>
> 
> </RDF>
>diff --git a/suite/themes/modern/install.rdf b/suite/themes/modern/install.rdf
>--- a/suite/themes/modern/install.rdf
>+++ b/suite/themes/modern/install.rdf
>@@ -32,12 +32,23 @@
>     <em:appManaged>true</em:appManaged>
> 
>     <em:locked>true</em:locked>
> 
>     <!-- Front End Integration Hooks (used by Theme Manager)-->
>     <em:creator>Mozilla Contributors</em:creator>
>     <em:contributor>Mozilla Contributors</em:contributor>
> 
>+    <em:localized>
>+      <Description>
>+        <em:locale>fi</em:locale>
>+        <em:name>Moderni</em:name>
>+        <em:description>Nykyaikainen teema kaikille ohjelmakomponenteille.</em:description>
>+        <em:creator>Mozilla-projektin tukijat</em:creator>
>+        <em:contributor>Mozilla-projektin tukijat</em:contributors>
>+        <em:translator>Aki Laaksovirta</em:translator>
>+      </Description>
>+    </em:localized>
>+
>     <em:internalName>modern/1.0</em:internalName>
>   </Description>
> 
> </RDF>

Comment 3

[Aki Laaksovirta]

Attachment: The Finnish description strings 2.0

Comment 4

[Axel Hecht [:Pike]]

I'd suggest to just make them localizable by prefs like we do in fx, http://mxr.mozilla.org/comm-central/source/mozilla/browser/app/profile/firefox.js#210

Comment 5

[Aki Laaksovirta]

What is the "[good first bug]" whiteboard status doing here? I have filed at least 5 bugs before this one.

Comment 6

[David Tenser [:djst]]

Aki, it just means that it's a good bug for anyone who wants to start contributing code to Mozilla because it's relatively easy to fix. We mark bugs with this to allow new code contributors to easily find bugs to get started with.

Comment 7

[Axel Hecht [:Pike]]

There's a bit of implicit communication here, beyond that. Philip moving this bug to preferences probably implies his support for my suggestion in comment 4, which would obsolete the patch that you already attached.

Feel free to propose a patch implementing the prefs way to localize the theme description. https://developer.mozilla.org/en/Localizing_extension_descriptions and the original fx patch in http://hg.mozilla.org/mozilla-central/rev/432fb39d5655 should be a good starting point.

Comment 8

[Aki Laaksovirta]

What advantage do we get by implementing this as a pref? "Because Firefox already has it that way" is simply not a reason good enough. And, as you can see, the patch that has been landed in mozilla-central changes entries in two files. Even that is too much if we can do this by changing entries in just one file. Additionally, modifying install.rdf is the more correct way in this case, since  prefs were originally created for quite a different use.

Comment 9

[Philip Chee]

> What advantage do we get by implementing this as a pref?

Doing it the Firefox way makes it easier for Localizers who then only have to remember one way of doing things on Firefox/Thunderbird/SeaMonkey

> And, as you can
> see, the patch that has been landed in mozilla-central changes entries in two
> files.
Yes but once done localizers in all locales don't have to notice that they have to localize install.rdf. Instead they can use their normal tools to localize the properties file which they know how to do. FWIW Babelzilla uses this method to localize extension descriptions as well.

Comment 10

[Aki Laaksovirta]

> Doing it the Firefox way makes it easier for Localizers who then only have to
> remember one way of doing things on Firefox/Thunderbird/SeaMonkey

Sorry, but SeaMonkey is different. We rely on the "right method for the right purpose" principle rather than some nasty hack.
 
> Yes but once done localizers in all locales don't have to notice that they have
> to localize install.rdf. Instead they can use their normal tools to localize
> the properties file which they know how to do. FWIW Babelzilla uses this method
> to localize extension descriptions as well.

So? Localizing install.rdf would be, as I said, the correct way to do this. The localizers wouldn't necessarily have to do this by themselves, they could just be asked to post the translations on .l10n and we could do the rest. FWIW Adblock Plus uses the correct method to localize extension descriptions as well. BTW the bug that this bug was made to depend on has been fixed for a long time.

Comment 11

[Robert Kaiser]

The right thing is for localizers NEVER to have to touch any of those nasty RDF files. getting it into .properties in some way is the only right way in the current Mozilla setup.

Comment 12

[Aki Laaksovirta]

Ah, well. I don't actually know what is the right way in this case. Sorry about the flaming.

Comment 13

[Aki Laaksovirta]

Attachment: patch for making theme descriptions localizable

This should do it. Does this one need any reviews?

Comment 14

[Jens Hatlak (:InvisibleSmiley)]

(In reply to comment #13)
> Does this one need any reviews?

Everything does. Try iann_bugzilla.

Comment 15

[Philip Chee]

Comment on attachment 512174 [details] [diff] [review]
patch for making theme descriptions localizable

Hmm. For L10n I'd think [e-mail] would be a better reviewer.

Comment 16

[Robert Kaiser]

Comment on attachment 512174 [details] [diff] [review]
patch for making theme descriptions localizable

Interesting, didn't know this worked that way, but after looking it up through the original bug etc. this sounds like the way we want to do it from inside the application, yes.

Thanks for the patch!

Comment 17

[Philip Chee]

Pushed to comm-central
http://hg.mozilla.org/comm-central/rev/b5bc77e944d2