905021 - [B2G][Shira-51007] Incorrect value sent for Accept-Language header on all http requests

Status: RESOLVED WONTFIX

(Firefox OS Graveyard :: General, defect, P1)

Description

[sync-1]

AU_LINUX_GECKO_ICS_STRAWBERRY.01.01.00.019.184
 Firefox os  v1.1
 Mozilla build ID:20130806071254
 
 DEFECT DESCRIPTION:
 Incorrect value sent for Accept-Language header on all requests
 
  REPRODUCING PROCEDURES:
 Description: Incorrect value sent for Accept-Language header on all requests.
 
 If the device has been configured to use e.g. German language (Germany):
 Accept-Language: de, de-de;q=0.8,de;q=0.6,en-us;q=0.4,en;q=0.2 
 -> primary tag de (without subtag) is given twice with different qvalue and primary tag de without subtag has a higher qvalue than the primary tag de with subtag de
 
 Header should be e.g.:
 Accept-Language: de-de, de;q=0.8; en-us;q=0.4,en;q=0.2
 
  EXPECTED BEHAVIOUR:
 
  ASSOCIATE SPECIFICATION:
 
  TEST PLAN REFERENCE:
 
  TOOLS AND PLATFORMS USED:
 
  USER IMPACT:
 
  REPRODUCING RATE:
 100%
 
  For FT PR, Please list reference mobile's behavior:

Comment 1

[Vicamo Yang [:vicamo][:vyang]]

That's not of a specific device, we don't have Accept-Language header included in MMS WSP/HTTP requests.

Comment 2

[Vicamo Yang [:vicamo][:vyang]]

The Accept-Language header is inserted in nsHttpHandler[1] and is controlled by pref "intl.accept_languages".  In B2G, we watch Settings entry "language.current" and update "intl.accept_languages" accordingly[2].  The RegExp match function might miss a few corner cases, but with the example for how server react with Accept-Language field in RFC 2616[3], I can't really feel that's something seriously wrong.

Reset bug summary and component for this's not handled in MMS at all.

[1]: http://mxr.mozilla.org/mozilla-central/source/netwerk/protocol/http/nsHttpHandler.cpp#1245
[2]: http://mxr.mozilla.org/mozilla-central/source/b2g/chrome/content/settings.js#73
[3]: http://www.w3.org/Protocols/rfc2616/rfc2616-sec14.html#sec14.4

Comment 3

[Joe Cheng [:jcheng] (please needinfo)]

minus the bug. changing http requests is not for v1.1 timeframe.
can the reporter be more specific on whether this cause any certification failures? Thanks

Comment 4

[xiupinglong]

(In reply to Joe Cheng [:jcheng] from comment #3)
> minus the bug. changing http requests is not for v1.1 timeframe.
> can the reporter be more specific on whether this cause any certification
> failures? Thanks

Hi, I have asked our val partner, she told me that this issue do not affect GCF certification. But it affect user experience, e.g. in German.

Comment 5

[Vicamo Yang [:vicamo][:vyang]]

(In reply to xiupinglong from comment #4)
> Hi, I have asked our val partner, she told me that this issue do not affect
> GCF certification. But it affect user experience, e.g. in German.

In MMS/SMS app? AFAIK, Accept-Language affects only X-Mms-Status-Text and similar ones returned from MMS Proxy-Relay.  But all these text strings are never passed to Gaia, let alone affect user experience.

Comment 6

[Jack Liu]

Since it's partner requirement, mark koi?.

Comment 7

[Julien Wajsberg [:julienw]]

Vicamo> on some carrier, if the carrier detects that the device doesn't support MMS (for example, if we support MMS basic and we need to receive a video), we get only a text message (I don't know if that's SMS or MMS) from the carrier. Therefore, maybe that language header is used to generate the text message in that situation.

On my unagi, I already had this message (as we don't send an UA profile) but I don't exactly remember if I had a message in english or french :/ I can try to reproduce though.

Comment 8

[Joe Cheng [:jcheng] (please needinfo)]

how big of a change will this require? it seems like this will change the http request format of the platform, which seem like a big change.
Vicamo/Julien?

Comment 9

[Julien Wajsberg [:julienw]]

Well, I just tried going to http://www.reliply.org/tools/requestheaders.php with my phone, and it tells me the Accept-Language header contains "fr". So this looks good to me.

Comment 10

[Joe Cheng [:jcheng] (please needinfo)]

just to confirm, on a v1.1 build?

Comment 11

[Julien Wajsberg [:julienw]]

yep

Comment 12

[Julien Wajsberg [:julienw]]

Attachment: mms.pcap

here is a pcap of a mms session. We can clearly see the "fr" value is sent in the Accept-Language header.

Comment 13

[Cheng-An, XIONG]

(In reply to Julien Wajsberg [:julienw] from comment #12)
> Created attachment 792780 [details]
> mms.pcap
> 
> here is a pcap of a mms session. We can clearly see the "fr" value is sent
> in the Accept-Language header.

Seems that the bug is misunderstood. Please double check.

Issue:
 Incorrect value sent for Accept-Language header on all requests.

Details:
 If the device has been configured to use e.g. German language (Germany):
 Then  Accept-Language: de, de-de;q=0.8,de;q=0.6,en-us;q=0.4,en;q=0.2

 (1) primary tag de (without subtag) is given twice with different qvalue, and
 (2) primary tag de without subtag has a higher qvalue than the primary tag de with subtag de

Expect:
 Header should be e.g.:
 Accept-Language: de-de, de;q=0.8; en-us;q=0.4,en;q=0.2

Comment 14

[Julien Wajsberg [:julienw]]

ok, this is definitely not a leo bug then. Won't block the release for this.

Comment 15

[Julien Wajsberg [:julienw]]

Cheng-An, do you know if this happens on Firefox Desktop too ?

Comment 16

[xiupinglong]

I found the function "nsresult
nsHttpHandler::SetAcceptLanguages(const char *aAcceptLanguages)" is called 2 times when I change my system language to German language (Deutsch).

The first time, the result is "de-de,de;q=0.8,en-us;q=0.5,en;q=0.3", (accept-language-change-to-de.png)

and the second time, the result become to "de, de-de;q=0.8,de;q=0.6,en-us;q=0.4,en;q=0.2".(accept-language-calcute.png)

The second time the languages number become 5, but in the first time is 4.

Comment 17

[xiupinglong]

And the "de" appears two times in the second time calcultion result.

Comment 18

[xiupinglong]

Attachment: accept-language-change-to-de.png

Comment 19

[xiupinglong]

Attachment: accept-language-calcute.png

Comment 20

[Julien Wajsberg [:julienw]]

xiupinglong> do you know where it is called ?

Comment 21

[xiupinglong]

(In reply to Julien Wajsberg [:julienw] from comment #20)
> xiupinglong> do you know where it is called ?

when you change language in settings->language, from English to Deutsch. It will be called. The NotifyObserver() notify the Observe(), then called PrefsChanged to SetAcceptLanguages().

You can find this using gdb or ddd.

Comment 22

[xiupinglong]

(In reply to Julien Wajsberg [:julienw] from comment #15)
> Cheng-An, do you know if this happens on Firefox Desktop too ?

I found Firefox Desktop has not this issue. When I add German/Germany[de-de], German[de], [en-us], [en],
The accept-language is "de-de,de;q=0.8,en-us;q=0.5,en;q=0.3".

Comment 23

[xiupinglong]

Attachment: accept-language-desktop.png

Comment 24

[Julien Wajsberg [:julienw]]

Can you please give us your /shared/resources/languages.json on this build ?

Comment 25

[Julien Wajsberg [:julienw]]

I'd say this comes from [1]. The rationale for this code is bug 796079.

Waiting for the languages.json file before moving forward, to know what is being set here.

[1] https://mxr.mozilla.org/mozilla-central/source/b2g/chrome/content/settings.js#93

Comment 26

[xiupinglong]

(In reply to Julien Wajsberg [:julienw] from comment #24)
> Can you please give us your /shared/resources/languages.json on this build ?

I have sent the file to your email, please check it. Thanks!

Comment 27

[Julien Wajsberg [:julienw]]

so, german is just "de" in the file.

Pike, can you explain why you chose to set the intl.accept_languages in bug 796079 in the end? I don't really know/follow all the language code in gecko.

Comment 28

[Julien Wajsberg [:julienw]]

it's better with the actual need info.

Comment 29

[Joe Cheng [:jcheng] (please needinfo)]

mark the bug confidential before partner sharing their private build information

Comment 30

[Axel Hecht [:Pike]]

So:

gaia's webl10n uses navigator.language to pick the language. https://github.com/mozilla-b2g/gaia/blob/master/shared/js/l10n.js#L1154

There's no fancy language tag matching there.

navigator.language is the first entry in the value of accept-lang. http://mxr.mozilla.org/mozilla-central/source/dom/base/Navigator.cpp#330

Thus, to get the language selection in gaia to work, our UI language needs to be the first item, more or less literally. We ignore the weight, but that's it.

Thus the gaia settings mapper prepends our value to the localized value we get from toolkit.

Bug 889335 would allow us to actually go through the full list, which may or may not be what we want. Some localized values might be off. But in this case, it'd avoid doubling the effort.

We can also parse the pref value, remove the given locale, and insert it at front in settings.js, I guess.

Comment 31

[Julien Wajsberg [:julienw]]

Pike, I still don't understand what getComplexValue in [1] is doing ?

If getComplexValue already put the de-de and de in the "intl" value, then why do we append something ourselves ?


[1] https://mxr.mozilla.org/mozilla-central/source/b2g/chrome/content/settings.js#83

Comment 32

[Axel Hecht [:Pike]]

The localized value is from gecko, and may be anything. We only *pre*pend if necessary, too. I.e., the German value is 

de-de, de, en-us, en

So to get 'de' first, we need to prepend 'de'. We could drop the other 'de', though.

http://mxr.mozilla.org/l10n-mozilla-aurora/search?string=intl.accept&find=intl.properties%24 for the full variant of values.

Comment 33

[Julien Wajsberg [:julienw]]

Pike: but do you think it's normal that we prepend this ? I mean, "de" is not the same as "de-de" (as "fr" is not the same as "fr-ca").

I thought that we could 

Also, you suggest that this bug is not resolvable before bug 889335 is fixed too ?

I'd say that about "Firefox OS inability to fallback from es-CA, to es, and then to en-US etc." (bug 889335 comment 7) it should be not very difficult to at least fallback from xx-yy to xx, and then to en-US as we already do.

If we let l10n.js do this, then we could simply _not_ prepend the language if it matches /^xx(?:[;-,]|$)/.

Comment 34

[Axel Hecht [:Pike]]

There's no reason to assume that the gecko version for a locale sports the locale before another locale supported on the device. At least I'd not think it necessarily had to be a bug outside of firefox os, which is different to our other Firefox products.

On fx os, web language preference is the same as UI language preference, which is somewhat a bug and a feature.

Thus, we can't in general stay away from manipulating the localized accept.lang value for fx os.

Comment 35

[Julien Wajsberg [:julienw]]

Pike> I wasn't suggesting to stay away from manipulating this value, but rather not manipulate it if starts with the correct language, where "correct language" means "choosen language" and its subtypes (eg: "de-de" and "de-at" for "de"). Which would need a modification in l10n.js to work.

If this path looks good I can try to do a patch for both files.

Comment 36

[Joe Cheng [:jcheng] (please needinfo)]

koi? as it seem like there can be some patches coming

Comment 37

[Axel Hecht [:Pike]]

I agree that we can reduce the impact of the tweaks here.

Mind, some are tricky, like zh-CN vs zh-TW might not be OK to mix, similarly for locales with encodings, like sr-Cyrl and sr-Latn.

Comment 38

[Julien Wajsberg [:julienw]]

For these locales, we keep the full locale name as the navigator.language, right ?

Comment 39

[Axel Hecht [:Pike]]

Yeah.

I'm not sure how to reliably do that, because you should probably only ignore values for which we don't have a locale in the gaia apps. Which is kinda ugh?

Comment 40

[xiupinglong]

Dears,

Any news about this issue?

Comment 41

[Julien Wajsberg [:julienw]]

Not yet, I haven't even started to produce a patch and nobody else came.

Comment 42

[bhavana bajaj [:bajaj]]

needinfo on Julien here :

Julien this seems to have the FX for 1.2 here. So what are the next steps here ? Is this something that will be proritized for 1.3 ?

Comment 43

[Julien Wajsberg [:julienw]]

I think that this bug, even if it is a real bug, has a very small user impact.

The patch should be quite easy to do, but we also need a (simple as well) patch in l10n.js.

Just not the time to do it right now, but if we prioritize it for 1.3 then we can do it fairly easily.

Comment 44

[Jason Smith [:jsmith]]

At this point, this likely isn't happening for 1.3, as this is not a committed feature for 1.3. I'm clearing the nom.

Comment 45

[BMO Automation]

Firefox OS is not being worked on