1215094 - add-on names are missing on production

Status: VERIFIED FIXED

(Marketplace Graveyard :: Reviewer Tools, defect, P1)

Description

[Andrew Williamson [:eviljeff]]

The names of the add-ons submitted after the 2015-10-13 push are missing in the review tools queue. If I get to the review page by inspecting the html page source its missing there too. 

Can't tell about consumer pages - the fact its not missing for all pending indicates it might be missing in the database.

Comment 2

[Andrew Williamson [:eviljeff]]

Based on the data in: https://gist.github.com/jasonthomas/2ce1575dc17fac0824ae

My guess would be that the uploaders are either using zh-TW locale Marketplace, or their requests are otherwise indicating that they are using the locale.  Two problems then:

1) The API during submission is then interpreting the locale of the manifest as being zh-TW, even though the manifest is entirely clean of locale information (rather than en-US) and
2) After saving the name as zh-TW localisation, the API during review isn't returning the only name we have.

Note webextension manifests don't have a default_locale field and rely on an entirely different l10n/i18n infrastructure: https://developer.chrome.com/extensions/i18n (i.e. if there is a default_locale field, we should ignore it)

Comment 3

[Mathieu Pillard [:mat]]

The i18n infrastructure in webextensions *is* using a default_locale field actually, which we should be parsing currently, even though we don't support the rest of the infrastructure.

We should use that default_locale field (which we transform in a language and set as default_language on the Extension instances) for incoming/outgoing translations, so there is a bug somewhere. I'll have a look.

Comment 4

[Andrew Williamson [:eviljeff]]

(In reply to Mathieu Pillard [:mat] from comment #3)
> The i18n infrastructure in webextensions *is* using a default_locale field
> actually, which we should be parsing currently, even though we don't support
> the rest of the infrastructure.
> 
> We should use that default_locale field (which we transform in a language
> and set as default_language on the Extension instances) for
> incoming/outgoing translations, so there is a bug somewhere. I'll have a
> look.

I'm not sure we should be using that field though.  According to the spec [1] "This field is required in extensions that have a _locales directory; it must be absent in extensions that have no _locales directory." - as we don't support the _locales directory and the `__MSG_messagename__`-style [2] key lookup then its always invalid for us.

example1:
{ 
  "name": "No1 Add-on",
  "default_locale": "en-US",
  ...
}
is not a valid manifest.

example2:
{ 
  "name": "__MSG_addonname__",
  "default_locale": "en-US",
  ...
}
with a _locales directory, and a messages.json file within with a "addonname" message inside, would be - but we don't support that yet.

[1] https://developer.chrome.com/extensions/manifest/default_locale
[2] https://developer.chrome.com/extensions/i18n

Comment 5

[Mathieu Pillard [:mat]]

Ah. It's too bad, it's useful information even in the absence of _locales, but yeah, let's raise a validation error when the field is used for now then. (we should still keep the existing model logic, it's good to have for the future - I've submitted a pull request to fix the empty names issue).

Comment 6

[Mathieu Pillard [:mat]]

Fixed in https://github.com/mozilla/zamboni/commit/a69deffed570b9b46b096e27e3f8ccdc340e41f6

That includes a migration to fix existing add-ons.

STR:
- Submit an add-on with your browser set to prefer a language that is not english
- With another browser set in english, try to review add-ons, the add-on you submitted should show up and its name should not be empty.

Comment 7

[krupa raj[:krupa]]

I submitted an add-on using "de" as my default language on dev and made sure the name shows up. See https://www.dropbox.com/s/g1f853b6of4ozcn/Screenshot%202015-10-19%2011.06.50.png?dl=0

On stage, where the patch hasn't landed yet, the same add-on ended up without a name. See the last add-on at https://www.dropbox.com/s/u6ezggor4vwqx8n/Screenshot%202015-10-19%2011.07.22.png?dl=0