1091803 - Android Nightly sends a weird Accept-Language header

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: Locale switching and selection, defect)

Description

[Tom S [:evilpie]]

When I got to http://httpbin.org/headers on a current nightly build for Android, the Accept-Language header says something like "chrome://global/locale/intl.properties,en-us;q=0.7,en;q=0.3".

Comment 1

[Richard Newman [:rnewman]]

This is https://bugs.launchpad.net/ubuntu/+source/firefox/+bug/867753, Bug 654099.

Comment 2

[Richard Newman [:rnewman]]

12:28:48 <@rnewman> evilpie: do you use Ubuntu for desktop?
12:29:29 < evilpie> rnewman: yes
12:30:34 < evilpie> interesting, I indeed use sync

Comment 3

[Richard Newman [:rnewman]]

… but we don't sync prefs, so this is my confused face. We don't read that pref as a simple string.

Comment 4

[Aaron Train [:aaronmt]]

Nightly (en-US, system-default (English), new profile) value for me: "en-us, en ; q=0.5"

Comment 5

[Richard Newman [:rnewman]]

Yeah, it worked when I landed it :)

We can band-aid this, but I'm curious to know how it could have occurred.

Comment 6

[Richard Newman [:rnewman]]

Attachment: Correctly treat general.useragent.locale and intl.accept_languages as complex prefs in all cases. v1

I think this is the right fix.

The problem was that general.useragent.locale is a localized pref *sometimes*. Android Nightly yes, desktop Nightly no, custom Android builds maybe.

So we'd read it when we found out what our OS locale was, and try to compute an Accept-Languages header from :

  OS locale = "en-US"
  App locale = "chrome://..."

The fix is to read g.u.l as a localized pref first. We also decide to write it as such, and to write intl.accept_languages as a localized pref, too.

This patch also includes a clause at the end of BrowserApp startup. This simply checks whether the OS locale has been set (i.e., this is not first run), then whether we have a malformed set of values. If so, it cleans them up.

We can kill this clause in the next release.

Comment 7

[Richard Newman [:rnewman]]

Note that this has a test! … and it passes because we're not running on multilocale builds on automation.

Bug 946058.

Comment 8

[Mark Finkle (:mfinkle) (use needinfo?)]

Might be worth filing a bug to remove the code in the next cycle?

Comment 9

[Richard Newman [:rnewman]]

https://hg.mozilla.org/integration/fx-team/rev/570921b6c566

Comment 10

[Richard Newman [:rnewman]]

ni me for uplift.

evilpie, this'll merge into Nightly some time over the weekend. Could you verify that things are fixed on Monday?

Comment 11

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/570921b6c566

Comment 13

[Markus Popp]

Works fine in today's (2014-11-03) Nightly. I think this can be uplifted to Aurora.

Comment 14

[Richard Newman [:rnewman]]

Thanks for verifying, Markus!

Comment 15

[Richard Newman [:rnewman]]

Comment on attachment 8514726 [details] [diff] [review]
Correctly treat general.useragent.locale and intl.accept_languages as complex prefs in all cases. v1

Approval Request Comment
[Feature/regressing bug #]:
  Bug 1045053.

[User impact if declined]:
  Crazy Accept-Language headers for all users.

[Describe test coverage new/current, TBPL]:
  Test coverage and local development testing in the original bug doesn't detect the problem because our infra builds don't match reality that well. Manually tested and verified on Nightly.

[Risks and why]: 
  Moderate risk, but the fixup code is well isolated, and the corrected code passes visual inspection. No alternative.

[String/UUID change made/needed]:
  None.

Comment 16

[Tom S [:evilpie]]

Fixed for me as well, thanks!

Comment 17

[Kevin Brosnan [Ex-Mozilla]]

This is verified.

Comment 18

[Richard Newman [:rnewman]]

https://hg.mozilla.org/mozilla-aurora/rev/9e6ac31cd86c