Android Nightly sends a weird Accept-Language header

RESOLVED FIXED in Firefox 35

Status

()

RESOLVED FIXED
4 years ago
2 years ago

People

(Reporter: evilpie, Assigned: rnewman)

Tracking

(Blocks: 1 bug)

unspecified
Firefox 36
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox35 fixed, firefox36 verified, fennec35+)

Details

Attachments

(1 attachment)

(Reporter)

Description

4 years ago
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".
Assignee: nobody → rnewman
Component: General → Locale switching and selection

Updated

4 years ago
tracking-fennec: --- → ?
status-firefox35: --- → ?
status-firefox36: --- → affected
(Assignee)

Comment 2

4 years ago
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
(Assignee)

Comment 3

4 years ago
… but we don't sync prefs, so this is my confused face. We don't read that pref as a simple string.
Nightly (en-US, system-default (English), new profile) value for me: "en-us, en ; q=0.5"
(Assignee)

Comment 5

4 years ago
Yeah, it worked when I landed it :)

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

Updated

4 years ago
Blocks: 1045053
(Assignee)

Comment 6

4 years ago
Created attachment 8514726 [details] [diff] [review]
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.
Attachment #8514726 - Flags: review?(mark.finkle)
(Assignee)

Comment 7

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

Bug 946058.
Attachment #8514726 - Flags: review?(mark.finkle) → review+
Might be worth filing a bug to remove the code in the next cycle?
(Assignee)

Updated

4 years ago
tracking-fennec: ? → 35+
status-firefox35: ? → affected
(Assignee)

Updated

4 years ago
Blocks: 1092421
(Assignee)

Comment 10

4 years ago
ni me for uplift.

evilpie, this'll merge into Nightly some time over the weekend. Could you verify that things are fixed on Monday?
Flags: needinfo?(rnewman)
Flags: needinfo?(evilpies)
https://hg.mozilla.org/mozilla-central/rev/570921b6c566
Status: ASSIGNED → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 36
(Assignee)

Updated

4 years ago
Duplicate of this bug: 1092886

Comment 13

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

Comment 14

4 years ago
Thanks for verifying, Markus!
Flags: needinfo?(rnewman)
(Assignee)

Comment 15

4 years ago
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.
Attachment #8514726 - Flags: approval-mozilla-aurora?
(Reporter)

Comment 16

4 years ago
Fixed for me as well, thanks!
Flags: needinfo?(evilpies)
Attachment #8514726 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
status-firefox36: affected → verified
status-firefox36: verified → fixed
This is verified.
status-firefox36: fixed → wontfix
status-firefox36: wontfix → verified
(Assignee)

Updated

4 years ago
status-firefox35: affected → fixed
(Assignee)

Updated

4 years ago
Blocks: 1130254
You need to log in before you can comment on or make changes to this bug.