1488516 - support multiple locales for search engines

Status: RESOLVED FIXED

(WebExtensions :: Frontend, enhancement, P1)

Description

[Shane Caraveo (:mixedpuppy)]

We're moving all locals for an engine into a single webextension since they are localizable.

An issue was brought up in bug 1486820 where the Malaysian distribution includes both the English and Malaysian Wikipedia search engines.

Secondary to that is potentially an ability to choose what locale is displayed for an engine.  For example, and engine that supports EN and RU installed into a JP distribution.  I'm inclined to default to EN, but should users have a way to select RU?

Comment 1

[Shane Caraveo (:mixedpuppy)]

One option to support having an english engine listed as a secondary engine in the UI may be to use alternate_urls, but I haven't thought that through entirely.

Comment 2

[Shane Caraveo (:mixedpuppy)]

We need to get product and/or ux input on this particular issue, I'm not certain where to ask for a decision on this.  The choices I think are:

1. Only install one engine that only provides one locale
2. Where needed, supply more than one extension so we can install multiple locales
3. Support an alternate_locale setting in search_provider, and re-parse the manifest to pull out an additional locale specific engine.

I like #3 the best but it is the most work.

Comment 3

[Mike de Boer [:mikedeboer]]

I *think* I like 3) the best too, but can you show me an example of how that would look like? That way I can see how that may end up working and presented in the UI...

Comment 4

[Shane Caraveo (:mixedpuppy)]

(In reply to Mike de Boer [:mikedeboer] from comment #3)
> I *think* I like 3) the best too, but can you show me an example of how that
> would look like? That way I can see how that may end up working and
> presented in the UI...

You can try it now with the opensearch xml files.  Just install two of the same engine, but from different locales.

You end up with two icons for the engine, with the titles in whatever locale they are.

Comment 5

[Shane Caraveo (:mixedpuppy)]

Being able to pull the manifest in different locale settings is a requirement now.  This is to support regionOverrides (see list.json).

The question is still whether we install more than one search engine per extension.

Comment 6

[Shane Caraveo (:mixedpuppy)]

The general plan here is:

- create extension.getLocalizedManifest(locale)
- create searchservice.addEnginesFromExtension
- refactor code in ext-chrome-settings-overrides that creates the details and calls searchservice
  - move code into searchservice.metaDataFromExtensionManifest

This will allow us to use list.json to determine what we need to do, such as loading multiple engines for different locales, choosing a different region, etc.  Those smarts already exist in searchservice and should continue to do so.

rough logic in searchservice will look something like:

addEngineFromExtension(extension) {
  let metadata = this.metaDataFromManifest(extension.manifest);
  this.addEngineFromMetaData(metadata)
  if extension.isPriviledged {
    if extension.id in lists.json.overrides {
      for overrideLocale in lists.json.overrides[extension.id] {
        localizedManifest = extension.getLocalizedManifest(overrideLocale)
        metadata = this.metaDataFromManifest(localizedManifest);
        this.addEngineFromMetaData(metadata)
      }
    }
  }
}

Comment 7

[Shane Caraveo (:mixedpuppy)]

Attachment: Bug 1488516 Add ability to get a localized copy of the manifest, r?aswan

This patch adds an ability to retreive a localized manifest without
otherwise changing anything.  This will be used by a later patch in 
nsSearchService that will allow multiple engines to be loaded from a 
single extension.

Comment 8

[Shane Caraveo (:mixedpuppy)]

Attachment: Bug 1488516 async-ify adding extension engines to the search service, r?rpl

We need to wait when adding engines to the search service so we can
finish up necessary logic after engines have been added.  This will allow us
to make async calls to fetch localized manifests in later patches.

Comment 9

[Shane Caraveo (:mixedpuppy)]

Attachment: Bug 1488516 support multiple regions per extension

This patch depends on bug 1496075.  Here we add an ability to load
multiple search engines based on details from list.json

Comment 10

[Mike Connor [:mconnor]]

Hey folks, we're going to have to go back to the drawing board on this front. Locales like en-US are not regions, and MUST NOT (in the RFC sense) be used to infer regions.  Don't even use it as a fallback.  Happy to explain in more detail offline.

As an existing example, en-US is the locale we currently offer to Canadian users, but we need to offer different URLs for Canadian users, i.e. for eBay, and maybe Amazon in the future.  Similarly, we have a bunch of en-US users in a lot of different countries that should get different options, including defaults (en-US users in Russia get Yandex).  If we're going to build something different, let's build something that maps to how search needs to work on a user and business level.

Structure-wise, that probably looks something like: (1 AM braindump follows)

In the add-on:

* global default URL for a provider
* a list of named URL variants for that provider
 * alternate_urls isn't quite right, array ordering would be too fragile
 * naming needs to be arbitrary, there's no perfect taxonomy or structure to be found

In list.json:

* list of defaults is set by provider, variants handled as overrides
* global default set (probably just Google, DDG, Wikipedia)
* region specific defaults
 * overrides the global default set if present
* region specific overrides
 * if the named variant isn't found, fall back to the default URL
* language specific overrides (which may override default or variants)
 * if not found, ignore and fall back.

{
  "global_defaults": {
     "default": "google",
     "visible": ["google","ddg","wikipedia"]
  },
  "regional_defaults": {
     "US": {
       "default": "google",
       "visible": ["google","ddg","bing","wikipedia","ebay","amazon"]
     },
     "CA": {
       "default": "google",
       "visible": ["google","ddg","bing","wikipedia","ebay","amazon"]
     },
     "DE": {
       "default": "google",
       "visible": ["google","ddg","bing","wikipedia","ebay","amazon"]
     }
  },
  "region_overrides": {
    "US": {
      "google": "google-2018"
    },
    "DE": {
      "amazon": "amazon-de", // provider -> override, uses amazon-de for all German users
      "ebay":   "ebay-de",
    },
    "CA": {
      "amazon": "amazon-ca", // provider -> override, uses amazon-de for all German users
      "ebay":   "ebay-ca",
    },
  },
  "languages": {
    "en-US": {
      "amazon-de": "amazon-de-en", // replace German with English
    },
    "fr-FR": {
      "amazon-ca": "amazon-ca-fr" // swap to French for those users
      "ebay-ca":   "ebay-ca-fr" 
    } 
  }
}


net would be:

US: ["google-2018","ddg","bing","wikipedia","ebay","amazon"]
CA: ["google","ddg","bing","wikipedia","ebay-ca","amazon-ca"]
CA (French): ["google","ddg","bing","wikipedia","ebay-ca-fr","amazon-ca-fr"]
DE: ["google","ddg","bing","wikipedia","ebay-de","amazon-de"]
DE (English): ["google","ddg","bing","wikipedia","ebay-de","amazon-de-en"]

didn't look at telemetry, but we'll want to make sure the name/variant gets reported there as the identifier, so we don't break reporting.

cc-ing Kaply, because I'm sure he'll also have thoughts, but man we have to be careful here.

Comment 12

[Shane Caraveo (:mixedpuppy)]

(In reply to Mike Connor [:mconnor] from comment #10)

Hey folks, we're going to have to go back to the drawing board on this
front. Locales like en-US are not regions, and MUST NOT (in the RFC sense)
be used to infer regions. Don't even use it as a fallback. Happy to
explain in more detail offline.

Lets do have that discussion, since I don't understand the distinction you're making here.

We're not inferring the users region from the locale, or at least nothing has changed in that aspect. I'm not familiar with the gritty details of how the search service decides on the region the user is in. To my understanding, we use the users locale as the base, then get the location (and thus region) they are in, and check to see if we need to override the defaults from their locale. IIUC We also update the regionOverrides, or maybe even the entire list.json, from some service.

What we are doing is using the l18n system already built into the webextension system as a way to deal with the various differences, which are in fact locale based. The search service can then override which locale is used base on the region the user is located in. The locale and override data from list.json already provided the capability to drive which locales are used to generate the manifest data from the extension to support all the current use cases contained in list.json.

The search service can also decide how it wants to failover if a given locale is not in the extension because the patches here provide a way to get a manifest in a specific locale. However, failover already works essentially like this:

1. user locale + override region if present
2. user locale
3. default locale defined by extension (unless extension has only one locale, then that)

#2 and #3 are simply how things work in extensions. #1 is driven by the search service.

For a en-US user traveling in CA, where CA has overrides, it would look like:

1. en-CA
2. en-US
3. whatever default_locale is in manifest

Given the above, a US user in CA could/would have different urls in extensions that provide an en-CA locale.

Google has a US specific search file, and another for everywhere else. Using the above, we would simply have en-US and en. Users traveling in the US would get the en-US locale if the override were:

regionOverrides: {
"US": { "google": "google-en-US" }
}

Likewise, currently an en-US user traveling to NL would get a different ebay url due to the NL override (exactly as-is in list.json):

"NL": {
"ebay": "ebay-nl"
},

Note: In the opensearch world, the above names are the filenames of the xml files. In the webextension world, the key is the extension name, the value is what we want to use. The value is parsed into the extension name and locale to be loaded from the extension.

Do you want to setup a meeting to discuss?

Comment 13

[Shane Caraveo (:mixedpuppy)]

Dale will take over D9136
MDB will look at D9135 and maybe integrate into the refactor he is doing.
That leaves D9134 for right now, which can land without the others, so lets see about getting review started there.

Comment 14

[Andrew Swan [:aswan]]

I'm not sure what you're asking me for?

Comment 15

[Shane Caraveo (:mixedpuppy)]

(In reply to Andrew Swan [:aswan] from comment #14)

I'm not sure what you're asking me for?

sorry, didn't realize that phab did not have you as reviewer.

Comment 16

[Mike Connor [:mconnor]]

As discussed in this morning's standup

• this is at least parity with what we've done before
• We'll have the flexibility to do more on the search service side as we move forward
• we're not going to be constrained by a * the use of locale codes if we need to handle weirder cases, e.g. en-US-de for an English version of a German site.
• we'll need to make sure at least the following items can be localized for variants:
  • search URLs
  • suggestion URLs (not discussed, but definitely true)
  • icons
  • display names
  • description text

So we're good to keep moving. I have an action to file a followup describing the business configuration requirements for the long term. That's looking like end of week.

Comment 17

[Shane Caraveo (:mixedpuppy)]

Attachment: Bug 1488516 document addEnginesFromExtension in nsISearchService.idl, r?mikedeboer

Comment 18

[Shane Caraveo (:mixedpuppy)]

https://hg.mozilla.org/projects/cedar/rev/14359eba64f5311658c6322e6f64a72ba041302e
Bug 1488516 document addEnginesFromExtension in nsISearchService.idl, r=mikedeboer

https://hg.mozilla.org/projects/cedar/rev/6479e91c5758e316bf9536d2f18a782442df8e15
Bug 1488516 Add ability to get a localized copy of the manifest, r=aswan

Comment 19

[Shane Caraveo (:mixedpuppy)]

This has merged from cedar, closing.

Comment 20

[Madalin Cotetiu]

Can you please add some STRs to this issue or mark it as "qe-verify- " if no manual testing is needed ?

Comment 21

[Shane Caraveo (:mixedpuppy)]

There's nothing on this that can be directly QEd, but the upcoming search changes (see bug 1517486) will likely have some QE elements.