394590 - Incorporate product-details into AMO

Status: RESOLVED FIXED

(addons.mozilla.org Graveyard :: Localization, defect)

Description

[Wil Clouser [:clouserw]]

We should find the best way to incorporate the product-details classes ( http://svn.mozilla.org/libs/product-details/ ) into AMO - especially the localeDetails class.  This will be useful because:

1) Any changes to locales we support (like changing their names) will happen across all our sites at once.  Consistency ftw

2) It will simplify the arrays we have to manage separately.  For example, the $native_languages array here: http://svn.mozilla.org/addons/trunk/site/app/config/language.inc.php

3) It will keep things like bug 394556 from happening, because we'll be copying the library on the backend instead of linking it on the front.

Comment 1

[Wil Clouser [:clouserw]]

I'm adding a dependency on bug 393634 because that bug is tracking the removal of download.old.js from mozilla.com.  Since AMO is depending on that file right now, we'll want to make sure this bug is fixed (ie. the dependency is gone) before the file disappears.

Comment 2

[Jesper Kristensen]

I still don't get why you want to use a static array for locales.

Also, the array only seems to support English and native. How would you make it localizable?

1 and 3 seems to me to be opposite goals. What exactly are you planning to do?

Why does this bug depend on bug 393634? Isn't it the other way around?

Comment 3

[Wil Clouser [:clouserw]]

(In reply to comment #2)
> I still don't get why you want to use a static array for locales.

I don't understand why it's a problem.

> 
> Also, the array only seems to support English and native. How would you make it localizable?

If we really want to support localizable names of languages we can always extend the product-details classes for something specific to AMO.  The point is, the classes are larger than AMO - they are used on several sites to keep our information consistent.

> 
> 1 and 3 seems to me to be opposite goals. What exactly are you planning to do?

I'm not following how they are opposite goals.  Right now we have things like "Spanish (South America)" on some sites and "Spanish (Argentina)" on other sites.  This will keep the names consistent across all sites.  How is that conflicting with (or even related to) removing an intersite dependency?

> 
> Why does this bug depend on bug 393634? Isn't it the other way around?
> 

*shrug* - I just wanted a link between the two bugs so we wouldn't forget there was a dependency.  Making the other bug depend on this one would probably be more correct.

Comment 4

[Robert Kaiser]

This sounds good. Will this also make the dictionaries table be created on the server-side instead of the client-side?

BTW, where should one file a bug/patch to extend the localeDetails array so that the multiple variants of German dictionaries will get proper titles when this has landed?

Comment 5

[Wil Clouser [:clouserw]]

(In reply to comment #4)
> This sounds good. Will this also make the dictionaries table be created on the
> server-side instead of the client-side?
> 

I think that's a good idea.  It's what we're doing for pages like this ( http://www.mozilla.com/en-US/firefox/all.html ).  Those pages used to be js, but are now generated via  PHP.

> BTW, where should one file a bug/patch to extend the localeDetails array so
> that the multiple variants of German dictionaries will get proper titles when
> this has landed?
> 

Yeah, we'll have to figure something out for that - I haven't thought through all the details yet.  If you want to file a bug about it, addons.mozilla.org::Localization would be the place.

Comment 6

[Jesper Kristensen]

(In reply to comment #3)
> (In reply to comment #2)
> > I still don't get why you want to use a static array for locales.
> 
> I don't understand why it's a problem.

Because when a dictionary is added to AMO, it usually takes months to fix its name.

> How is that conflicting with (or even related to) removing an intersite
> dependency?

Either I misread you, or "the classes are larger than AMO - they are used on several sites" conflicts with removing intersite dependency.

Comment 7

[Robert Kaiser]

Jesper:
Your "fix" to use the package descriptions itself is a problem, as I wouldn't want a dictionary to just be called "German" in the Add-Ons window of Firefox.

That it takes so long to fix those names is probably because we had/have language names and product locales intermingled in the old JS array, while the new way makes it possible to do this differently, which should make it much easier to get dictionary display fixed.

Comment 8

[Jesper Kristensen]

(In reply to comment #7)
> Jesper:
> Your "fix" to use the package descriptions itself is a problem, as I wouldn't
> want a dictionary to just be called "German" in the Add-Ons window of Firefox.

I know it is not very pretty to reuse a field intended to something else, but the description is used nowhere else, not even in the Add-on Manager window.

> That it takes so long to fix those names is probably because we had/have
> language names and product locales intermingled in the old JS array, while the
> new way makes it possible to do this differently, which should make it much
> easier to get dictionary display fixed.

If that is true, then everything is fine. But I would like to see it first.

Comment 9

[Wil Clouser [:clouserw]]

Attachment: use localeDetails info for $native_languages

This patch will incorporate product-details into AMO.  To test the patch you'll need to checkout http://svn.mozilla.org/libs/product-details/ into /site/vendors/product-details/ (since this will be an SVN external I can't include it in the patch).

All this patch currently does is replace the native_languages array with the one in locale_details, but fixing this bug will open up the door to fixing others (I'm thinking of our dictionary problems right now).  I think those problems are other bugs though.

Requesting review from anyone.

Comment 10

[Justin Scott [:fligtar]]

Comment on attachment 295979 [details] [diff] [review]
use localeDetails info for $native_languages

Everything still works!

Comment 11

[Wil Clouser [:clouserw]]

Thanks Justin.  This is r9602.  I'm adding push-needed to the keywords.  I'm not totally sure how this is going to work with how we are tagging for production (with the external), so whoever does that should double check things work right.

Comment 12

[Justin Scott [:fligtar]]

This is live.