424081 - Recommended add-ons is a 404 for Afrikaans (af)

Status: VERIFIED FIXED

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

Description

[Axel Hecht [:Pike]]

When trying to look at the recommended add-ons in the Afrikaans add-ons manager, I'm getting an XML parser error due to a 404 page in the error console:

Fout: wangepaste merker. Verwag: </link>.
Bronlêer: https://services.addons.mozilla.org/en-US/firefox/af/firefox/api/1/list/featured/all/10/WINNT/3.0b5pre
Reël: 75, Kolom: 3
Bronkode:
</head>

It works for en-US, though. https://services.addons.mozilla.org/af/firefox/api/1/list/featured/all/10/WINNT/3.0b5pre is the real URL, I guess this has something to do with amo not knowing about all locale codes we have.

Comment 1

[Wil Clouser [:clouserw]]

> I guess this has something to do with amo not knowing about
> all locale codes we have.
> 

Yep.  Would it be better to return English results or no results?  If no results, we could just make any 404s on services.amo return a blank page (might not be a bad idea anyway).

Comment 2

[Axel Hecht [:Pike]]

I'd prefer English results, that'd seemlessly work for lang-packs, too. Should I talk about accept_lang? That'd be perfect in that scenario.

Empty documents would probably just yield a differnt cumbersome error message in http://mxr.mozilla.org/mozilla/source/toolkit/mozapps/extensions/src/nsAddonRepository.js#324.

Btw, where do I check which list of languages amo currently knows about?

Comment 3

[Wil Clouser [:clouserw]]

(In reply to comment #2)
> I'd prefer English results, that'd seemlessly work for lang-packs, too. Should
> I talk about accept_lang? That'd be perfect in that scenario.

If the manager can handle 302s until it gets to the right page, we could talk about this.

> 
> Empty documents would probably just yield a differnt cumbersome error message
> in
> http://mxr.mozilla.org/mozilla/source/toolkit/mozapps/extensions/src/nsAddonRepository.js#324.
> 
> Btw, where do I check which list of languages amo currently knows about?
> 

The "Other Languages" list on the bottom right of AMO or for locale codes http://svn.mozilla.org/addons/trunk/site/app/config/language.inc.php

Comment 4

[Axel Hecht [:Pike]]

Let's see what Dave has to add, he's all over nsAddonRepository.js

Comment 5

[Dave Townsend [:mossop]]

The service already follows 302s automatically. I just tested by changing the url to https://services.addons.mozilla.org/api/1/list/featured. On opening the manager it received a 302 to https://services.addons.mozilla.org/en-US/api/1/list/featured followed by another 302 to https://services.addons.mozilla.org/en-US/firefox/api/1/list/featured.

Comment 6

[Laura Thomson :laura]

The API part of this is to always serve XML in response to API requests so the addons manager and other clients don't get confused.

Will open a separate bug for the redirection issue because that's a general issue with unknown locales on AMO, that's https://bugzilla.mozilla.org/show_bug.cgi?id=427792

Comment 7

[Wil Clouser [:clouserw]]

Attachment: add locale detection & redirection

This patch:

1) Adds opt-in HTTP_ACCEPT_LANG detection to AMO (woo!)
2) Fixes a detection problem.  Now if a valid* locale is specified in the URL, but AMO doesn't support it, we pop the valid-but-not-supported locale off the URL and redetect a suitable locale from the browser.  If all else fails, we give en-US.

This patch does not fix bug 427792 which is a broken page when an invalid locale is specified.

*valid is defined as being in the list here: http://svn.mozilla.org/libs/product-details/localeDetails.class.php

Comment 8

[Wil Clouser [:clouserw]]

Attachment: add locale detection & redirection

New patch does everything the last one did and also appears to fix bug 427792.  It passes all my testing and the unit tests.  Please hammer away. :)

Comment 9

[Laura Thomson :laura]

Comment on attachment 318075 [details] [diff] [review]
add locale detection & redirection

Good for me.

Comment 10

[Laura Thomson :laura]

Attachment: Patch for the API component of this.

Give a 404 in XML so the addons manager doesn't throw a bug.

Also, symlink the missing controller error to the 404, because Axel's sample url actually provokes this issue rather than 404ing.

Comment 11

[Wil Clouser [:clouserw]]

(In reply to comment #9)
> (From update of attachment 318075 [details] [diff] [review])
> Good for me.
> 

Thanks, r12648

Comment 12

[Fred Wenzel [:wenzel]]

Comment on attachment 318449 [details] [diff] [review]
Patch for the API component of this.

Looks good. Don't you want to replace the server name in there by SERVICE_URL or so?

Comment 13

[Wil Clouser [:clouserw]]

Comment on attachment 318449 [details] [diff] [review]
Patch for the API component of this.

/me r+'s again

Comment 14

[Laura Thomson :laura]

API stuff committed in r12656.

Didn't use the SERVICE_URL as that is for service tests according to morgamic.

Comment 15

[Stephen Donner [:stephend] Not actively reading bugmail]

Attachment: Screenshot of [af] add-ons manager

Does this screenshot prove it's fixed?

Comment 16

[Axel Hecht [:Pike]]

No, it proves that this isn't fixed yet, and https://services.addons.mozilla.org/af/firefox/api/1/list/featured/all/10/WINNT/3.0pre is a 404.

I guess this waits for a server push?

I tried to find a preview or stage for services, but failed.

Comment 17

[Wil Clouser [:clouserw]]

It looks like services.a.m.o didn't get updated last week.  If you change the URL from comment #0 to be addons.mozilla.org/... it works.  We'll update services.a.m.o this thursday.

Comment 18

[Stephen Donner [:stephend] Not actively reading bugmail]

https://services.addons.mozilla.org/en-US/firefox/api/1/list/featured/all/10/WINNT/3.0pre now works (2nd URL from comment 0), and I was told that's what I needed to verify, so Verified FIXED.