Android N in user agent is breaking some websites

RESOLVED FIXED in Firefox 48

Status

()

Core
Widget: Android
--
major
RESOLVED FIXED
a year ago
a year ago

People

(Reporter: mkaply, Assigned: mkaply)

Tracking

(Blocks: 1 bug)

Trunk
mozilla50
Points:
---

Firefox Tracking Flags

(firefox48+ verified, firefox49+ fixed, fennec50+, firefox50 verified)

Details

Attachments

(2 attachments)

(Assignee)

Description

a year ago
The Android N user agent for Firefox contains N as the version which appears to be breaking some websites.

Looking at Chrome, they use 6.0.99, not N.

We need to do the same.
(Assignee)

Comment 1

a year ago
Chrome grabs the major and minor versions explicitly:

https://cs.chromium.org/chromium/src/content/common/user_agent.cc?sq=package:chromium&dr=C&rcl=1467803755&l=90

https://cs.chromium.org/chromium/src/content/common/user_agent.cc?sq=package:chromium&dr=C&rcl=1467803755&l=53

We must be querying something different.
(Assignee)

Comment 2

a year ago
https://dxr.mozilla.org/mozilla-central/source/netwerk/protocol/http/nsHttpHandler.cpp#776

    rv = infoService->GetPropertyAsAString(
        NS_LITERAL_STRING("release_version"), androidVersion);

https://dxr.mozilla.org/mozilla-central/source/xpcom/base/nsSystemInfo.cpp#836

  if (mozilla::AndroidBridge::Bridge()->GetStaticStringField(
      "android/os/Build$VERSION", "RELEASE", str)) {
    aInfo->release_version() = str;
(Assignee)

Updated

a year ago
Blocks: 1255124
Severity: normal → major
(Assignee)

Comment 3

a year ago
Created attachment 8768592 [details] [diff] [review]
Potential fix

This is basically the same concept as Chrome, only we don't need to parse out all three values. We just need to make sure that we got at least one number from the parse.

See:

https://cs.chromium.org/chromium/src/base/sys_info_android.cc?l=70
Assignee: nobody → mozilla
Attachment #8768592 - Flags: review?(rnewman)
Comment on attachment 8768592 [details] [diff] [review]
Potential fix

Review of attachment 8768592 [details] [diff] [review]:
-----------------------------------------------------------------

Over to froydn for nsSystemInfo (or a 302 -- karlt?).

::: mobile/android/base/java/org/mozilla/gecko/SysInfo.java
@@ +185,5 @@
>      /**
>       * @return the release version string, such as "4.1.2".
>       */
>      public static String getReleaseVersion() {
> +        Log.w(LOG_TAG, android.os.Build.VERSION.RELEASE);

I think you missed this.
Attachment #8768592 - Flags: review?(rnewman) → review?(nfroyd)
Status: NEW → ASSIGNED
tracking-fennec: --- → ?
Component: General → Widget: Android
Product: Firefox for Android → Core
tracking-fennec: ? → 50+
Comment on attachment 8768592 [details] [diff] [review]
Potential fix

Review of attachment 8768592 [details] [diff] [review]:
-----------------------------------------------------------------

I have no comment on the SysInfo.java changes.  r=me with some comment changes noted below.

::: xpcom/base/nsSystemInfo.cpp
@@ +813,5 @@
>  }
>  
>  #ifdef MOZ_WIDGET_ANDROID
> +// Default version of Android to fall back to when actual version numbers
> +// cannot be acquired. Use the latest Android release with a higher bug fix

I know that this comment was cut-and-pasted from Chromium, but I think it'd be good to mention circumstances where this can happen--e.g. "some versions of Android return a non-numeric value for the version number".

@@ +815,5 @@
>  #ifdef MOZ_WIDGET_ANDROID
> +// Default version of Android to fall back to when actual version numbers
> +// cannot be acquired. Use the latest Android release with a higher bug fix
> +// version to avoid unnecessarily comparison errors with the latest release.
> +// This should be manually kept up to date on each Android release.

Does that mean that we have to constantly remember to update this each time we want to support a new Android release?  Is there any way to derive this from the environment?
Attachment #8768592 - Flags: review?(nfroyd) → review+
(Assignee)

Comment 6

a year ago
> Does that mean that we have to constantly remember to update this each time we want to support a new Android release?  Is there any way to derive this from the environment?

Sadly yes. There does not appear to be a way to get this from the environment (hence the reason Chrome hard codes it as well).

I'll update the comment.
(Assignee)

Comment 7

a year ago
// Prerelease versions of Android use a letter instead of version numbers.
// Unfortunately this breaks websites due to the user agent.
// Chrome works around this by hardcoding an Android version when a
// numeric version can't be obtained. We're doing the same.
// This version will need to be updated whenever there is a new official
// Android release.
// See: https://cs.chromium.org/chromium/src/base/sys_info_android.cc?l=61
(Assignee)

Comment 8

a year ago
Created attachment 8770604 [details] [diff] [review]
Final patch

For posterity, this is the final patch.

I moved the version to a "const" after I did my testing and somehow ended up writing as if it were Javascript.

This patches used a #DEFINE for the default android version and updates the comment.
(Assignee)

Comment 9

a year ago
https://hg.mozilla.org/integration/fx-team/rev/9efa66f1c6a15449be7a3c37ade8e104ce2afef8
Bug 1284921- Hardcode Android version for non-numeric releases. r=nfroyd

Comment 10

a year ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/9efa66f1c6a1
Status: ASSIGNED → RESOLVED
Last Resolved: a year ago
status-firefox50: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla50
(Assignee)

Comment 11

a year ago
[Tracking Requested - why for this release]: We have a partner who is prepping to release Firefox 48 Fennec possibly on Android N,

Having this fix in will allow them to test websites properly on prelease Android N with release Firefox 48.

Patch is low risk.
tracking-firefox48: --- → ?
tracking-firefox49: --- → ?
Per comment #11, track this on 48/49.
tracking-firefox48: ? → +
tracking-firefox49: ? → +
Mike, any reason why you don't ask for an uplift request?
Flags: needinfo?(mozilla)
status-firefox48: --- → affected
status-firefox49: --- → affected
(Assignee)

Comment 14

a year ago
Comment on attachment 8770604 [details] [diff] [review]
Final patch

Approval Request Comment
[Feature/regressing bug #]:
[User impact if declined]: Firefox won't function properly with Android N prerelease
[Describe test coverage new/current, TreeHerder]:
[Risks and why]: Low. No change on current Android.
[String/UUID change made/needed]:

> Mike, any reason why you don't ask for an uplift request?

I thought you were supposed to ask for tracking before uplift.

Requesting uplift.

Honestly, I'm quite confused when it comes to all the various flags.
Flags: needinfo?(mozilla)
Attachment #8770604 - Flags: approval-mozilla-release?
Attachment #8770604 - Flags: approval-mozilla-beta?
Comment on attachment 8770604 [details] [diff] [review]
Final patch

Review of attachment 8770604 [details] [diff] [review]:
-----------------------------------------------------------------

This patch fixes android version number. Take it in 48 beta 9. This should be fixed in fennec 48 beta 9.
Attachment #8770604 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Hi Mike,
Can you also create uplift request for 49 aurora?
Flags: needinfo?(mozilla)
https://hg.mozilla.org/releases/mozilla-beta/rev/2b7160a603f1
status-firefox48: affected → fixed
(Assignee)

Comment 18

a year ago
Comment on attachment 8770604 [details] [diff] [review]
Final patch

Correcting accidental release approval request to aurora
Flags: needinfo?(mozilla)
Attachment #8770604 - Flags: approval-mozilla-release? → approval-mozilla-aurora?
Comment on attachment 8770604 [details] [diff] [review]
Final patch

Review of attachment 8770604 [details] [diff] [review]:
-----------------------------------------------------------------

Per comment #15, take it in aurora.
Attachment #8770604 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Verified as fixed on latest Nightly(50.0a1 2016-07-19) and Beta (48.0b9) builds on a Nexus 6P with android 7.0. this issue was verified by accessing whatsmyua.com 

Still waiting for Aurora uplift.
status-firefox48: fixed → verified
status-firefox50: fixed → verified

Comment 21

a year ago
bugherderuplift
https://hg.mozilla.org/releases/mozilla-aurora/rev/5de1c40fd9bf
status-firefox49: affected → fixed
You need to log in before you can comment on or make changes to this bug.