This is also relevant in the vanishingly unlikely case that a Gingerbread user upgrades their phone OS to a newer Android.

Shifting ni from Bug 1117108.

Your suggestion from bug 1117108 for using a system notification that offered an option to "Download correct APK" was a good idea - let's start with that. A bit of noodling we'll have to figure out I guess this "download" button. It might have to lead to an "Open with..." notification since they'll have to use another browser to visit the download page. Suggested copy: Sorry! $APPNAME not supported for this device. Visit mozilla.org to download the correct version. Cancel Download

We should be able to trigger the download directly. We have the right permission (Bug 1109198).

Perfect - Sorry! $APPNAME not supported for this device. Please download the correct version. Cancel Download

Bug 1132046 would almost certainly have been flagged (if not prevented) by this.

enr0n: glad you're interested in this ticket. Some context: Bug 1039789 and friends got us to build two different Fennec APKs, one for Android API levels 9-10, one for Android API levels 11+. You can see the two different types being produced in automation at, say, https://treeherder.mozilla.org/#/jobs?repo=mozilla-central&revision=eab21ec484bb -- observe the B ("build") entries for "Android 2.3 API9 opt" and then "Android 4.0 API11+ opt". The API level 9-10 APK will work fine on newer devices. *The API level 11+ one won't work at all on older devices.* This ticket is to show a toast/dialog when the API level 11+ APK is used on an API 9-10 device. I'll include some code links to show where a check-and-message should go in a follow up comment.

enr0n: this code needs to happen /early/ in the startup process. I think somewhere in GeckoLoader.setupGeckoEnvironment is appropriate: https://dxr.mozilla.org/mozilla-central/source/mobile/android/base/mozglue/GeckoLoader.java#126. A great first attempt would create a notification in that code, /always/ show it (for testing), and then abort the app with logging. Then we can work out the details of testing the API range.

rnewman: you were working in this area. A different approach to https://bugzilla.mozilla.org/show_bug.cgi?id=1119915#c7 would be to make all the "log and return" blocks throw a particular exception, which we would then catch, log, (telemetry?), surface to the user (?), and offer the notification. Any thoughts on that?

(In reply to Nick Alexander :nalexander (PTO July 17-27) from comment #7) > A great first attempt would create a notification in that code, /always/ > show it (for testing), and then abort the app with logging. Then we can > work out the details of testing the API range. Great, I'll get working on it.

I think we can do earlier, shouldn't really tie this to starting Gecko (note that setupGeckoEnvironment is only called from GeckoThread.run), and we need to hook into the browser activity to stop further issues. That is: at any of the entry points into the browser (GeckoApp, tab queue, Sync setup, …), we should do a super cheap check like this: if (SDK_INT < MIN || SDK_INT > MAX) { showVersionError(); this.finish(); return; } That means we don't continue to run the activity (and possibly crash), we don't start Gecko at all (it might load the wrong libs), etc. Why not put this in GeckoApplication? Because we want to show UI and stop the individual activities from running. We'd only be able to do that by crashing in the constructor. Why not put this in GeckoLoader? Same answer, really: too little, too late.

This is a very crude first go -- I could certainly use some guidance. I chose the onCreate() method within GeckoApp.java to place the check for now, but what would be a better alternative?

Comment on attachment 8638872 [details] [diff] [review] first attempt Review of attachment 8638872 [details] [diff] [review]: ----------------------------------------------------------------- ::: mobile/android/base/GeckoApp.java @@ +128,5 @@ > > private static final String LOGTAG = "GeckoApp"; > private static final int ONE_DAY_MS = 1000*60*60*24; > + private static final int MINIMUM_SDK_INT = 11; > + private static final int MAXIMUM_SDK_INT = 22; // Clarify these values. You'll want to use AppConstants.MIN_SDK_VERSION and AppConstants.MAX_SDK_VERSION instead of these. @@ +1132,5 @@ > + // Ensure that the user is using the correct APK for their device's API. > + // If not, provide a way to get the correct version. > + if (Build.VERSION.SDK_INT < MINIMUM_SDK_INT || > + Build.VERSION.SDK_INT > MAXIMUM_SDK_INT) { > + showVersionError(this); You ought to finish the activity and return after showing this error. You should probably also lift this out into a method (that presumably returns a boolean so you know whether to abort from onCreate); you'll need to do so for reuse elsewhere. @@ +2696,5 @@ > } > + > + protected void showVersionError(final Activity activity) { > + final AlertDialog.Builder builder = new AlertDialog.Builder(this); > + builder.setMessage("Some message") We'll use R.string for this; take a look at mobile/android/base/locales/en-US/android_strings.dtd and mobile/android/base/strings.xml.in grep for something like "doorhanger_login_no_username" for an example of what to do. @@ +2700,5 @@ > + builder.setMessage("Some message") > + .setPositiveButton(R.string.button_ok, new DialogInterface.OnClickListener() { > + @Override > + public void onClick(DialogInterface dialog, int id) { > + activity.finish(); This will presumably allow GeckoApp to continue initializing until the user hits the button, right? If so, I think we need to go with my suggestion above, and end the activity immediately after showing the dialog and before continuing with init. Eventually we'll want this dialog to do something smart like kick us out to the correct place to download the right APK, but this is a good start.

(In reply to Richard Newman [:rnewman] from comment #12) > This will presumably allow GeckoApp to continue initializing until the user > hits the button, right? If so, I think we need to go with my suggestion > above, and end the activity immediately after showing the dialog and before > continuing with init. If I do something like this : if (!isValidSdk()) { showVersionError(); finish(); return; } The finish() is called before the dialog is actually shown. Actually, if I have the return statement I get a 'Unfortunately Fennec has stopped working' message. (If I don't have the return, the former occurs.) Not sure what I'm doing wrong here.

Try using `getApplicationContext()` as the context instead of `this` when you construct the AlertDialog.Builder -- you're constructing a dialog that's hanging off an activity that's immediately discarded. That might not work, because you might need a specific kind of Context, but it's worth a shot. Failing that, use a Toast instead of an AlertDialog. You'd need to post a log before I could take a worthwhile guess at why the return is causing a crash, but I would speculate that it's because `onDestroy` (which will be called after the return) is attempting to unregister components that were never registered. That should be an easy fix.

I think your speculation is correct about the crash -- I tried placing the check at the very end of onCreate() and there is no crash. This would be too late for the check though correct? I can still post the log of the original crash if you'd like. Also I tried using a Toast with LENGTH_LONG duration, but this also disappears too early to be of any use.

(In reply to Nicholas Rosbrook [:enr0n] from comment #15) > This would be too late for the check though correct? Yes. We don't want to finish onCreate, because it could cause version-dependent crashes as it initializes Gecko or loads more UI. > I can still post the log of the original > crash if you'd like. Please do. I think we want to spin out a dependency of this bug to make onDestroy not crash if we abort onCreate! > Also I tried using a Toast with LENGTH_LONG duration, but this also > disappears too early to be of any use. OK. Might need to show a toast and then fall back to a system notification.

Here is the original log.

That crash is dying in onCreate, which is… interesting. Implies that you didn't return out of onCreate.

What does this mean exactly then? I can post actual code for you to see, but what I have is the same as my above post where I originally described the crash.

Please post a patch, Nicholas. It'll be easier for me to figure out what's going on if I can repro.

Comment on attachment 8653221 [details] [diff] [review] bug-1119915-fix.patch Review of attachment 8653221 [details] [diff] [review]: ----------------------------------------------------------------- This looks like the right direction, so yeah, I bet this is crashing in onDestroy. Lemme see if I can repro. ::: mobile/android/base/GeckoApp.java @@ +5,5 @@ > > package org.mozilla.gecko; > > +import android.os.*; > +import android.os.Process; We don't need these.

This is becoming more of an issue. The prerelease channels (Aurora/Nightly) are especially problematic.

I am going to pick this up.

Bug 1119915 - Show toast if APK API range does not match device. r?

Comment on attachment 8682498 [details] MozReview Request: Bug 1119915 - Show toast if APK API range does not match device. r? This patch picks up the work from Nicholas and shows a toast if the device's SDK version does not match the APK API range. I wanted to implement the notification, pointing to the download, as well. But it turns out to be quite tricky: Sometimes we want to point to a website (https://www.mozilla.org/firefox/android/ or http://nightly.mozilla.org ?), sometimes we might want or can to point to the APK directly (Nightly, Aurora?) but we will have to download it with a different browser; and then sometimes we better link to the play store or other third-party store (Beta, Release) to guarantee that the user will get updates. An finally there might be custom/partner builds where we want to point to something totally different.

And sometimes we can't offer a download at all: if you're a Froyo user, or -- soon -- a Honeycomb user, we don't have anything for you. Bug 1220773 will answer some of your questions at runtime :) So this might be something like: if (installedFromPlay) { sorry(play); } else if (knownUnsupportedVersions) { sorry(old); } else { // TODO: make a request to AUS to find out if there's a correct APK for our parameters? goHere(https://ftp.mozilla.org/...) }

Comment on attachment 8682498 [details] MozReview Request: Bug 1119915 - Show toast if APK API range does not match device. r? https://reviewboard.mozilla.org/r/24077/#review21621 ::: mobile/android/base/GeckoApp.java:2145 (Diff revision 1) > + if (!isSupportedSDK()) { Smart hack. ::: mobile/android/base/GeckoApp.java:2256 (Diff revision 1) > + Build.VERSION.SDK_INT <= Versions.MAX_SDK_VERSION; One char to the left, please\! ::: mobile/android/base/locales/en-US/android_strings.dtd:757 (Diff revision 1) > +<!ENTITY unsupported_sdk_version "Sorry! &brandShortName; not supported for this device. Please download the correct version."> Wording tweak: > Sorry! This &brandShortName; won\'t work on this device. Please download the correct version. Extra points if you add placeholders here and pass in the ABI and SDK_INT: > <!-- Localization note: the formatS is replaced by the CPU ABI (e.g., ARMv7); the formatD is replaced by the Android OS version (e.g., 14). --> > Sorry! This &brandShortName; won\'t work on this device (&formatS;, &formatD;). Please download the correct version.

https://hg.mozilla.org/integration/fx-team/rev/1070b7e7836025bcd0cb84396fe090f9e5120fd0 Bug 1119915 - Show toast if APK API range does not match device. r=rnewman

I landed the toast with the improvements from comment 28. I filed bug 1222538 to continue with the notification and APK download.

Is it worth adding some telemetry to see how often this situation is hit? Maybe a followup.

Traditional telemetry for this isn't as easy as it might seem -- this escape hatch is hit when the APK includes code that doesn't work on this device. For example, we might have an x86 libxul on an ARM device, so we can't use Gecko's telemetry uploader. Or we might have v14+ API usage in a future Java implementation of telemetry upload, which will fail on the Java side when called on Honeycomb. So let's broaden the question: can we measure how often this situation is hit? I think we could: assume that users who care (which are the ones we want to measure!) will click through a notification, and let's make sure it does something useful _and_ measurable -- * Load a page in the stock browser. We can measure that with regular web analytics. * Load something else (e.g., Google Play) but either go via a bouncer or send some other kind of ping. We can tackle both of those in Bug 1222538.

Does what landed here already include warning x86 users that they have an ARM APK (or the other way round) or does that need additional work?

ni comment 35.

(In reply to Robert Kaiser (:kairo@mozilla.com) from comment #35) > Does what landed here already include warning x86 users that they have an > ARM APK (or the other way round) or does that need additional work? No. This just checks the Android version to detect if the wrong APK split has been installed. But checking the ABI makes sense of course. I thought I saw a bug about that in the past. However I could not find anything, so I filed bug 1222925. - Regarding telemetry: I copied the comment over to bug 1222538 so that we can try to solve this there.

ARMv7 API 11+ * Confirm that this functions on an ARMv7 API 11+ device with an API 9-10 build. * The x86 build crashes on start (mozilla crash reporter) when launched on an ARMv7 x86 (Motorola Razr i) * The API 9-10 build displays the dialog * The API 11+ build sometimes will start sometimes will crash back to the launcher without a dialog ARMv7 API 9-10 * API 11+ build is not unpacked by the OS. Gives a corrupt APK warning * The x86 build crashes on start (mozilla crash reporter) when launched on an ARMv7 "Sorry! This nightly won't work on this device (armeabi-v7a, 19) Please download the correct version." Does the wording of this dialog make sense? This nightly on this device is an odd phrase.

(In reply to Kevin Brosnan [:kbrosnan] from comment #38) > "Sorry! This nightly won't work on this device (armeabi-v7a, 19) Please > download the correct version." > Does the wording of this dialog make sense? This nightly on this device is > an odd phrase. It's overly specific, but I think that's a good thing. If we wanted the wording to feel less odd, we could use: ...This nightly won't work on your device...

(In reply to Sebastian Kaspari (:sebastian) from comment #37) > No. This just checks the Android version to detect if the wrong APK split > has been installed. But checking the ABI makes sense of course. I thought I > saw a bug about that in the past. However I could not find anything, so I > filed bug 1222925. Thanks, we had those cases where someone linked an ARM APK on a forum and people with x86 devices would install it from there.

Margaret, as I mentioned in https://bugzilla.mozilla.org/show_bug.cgi?id=1222925#c23, given that this has been an issue for us in the past few releases, I think it makes sense to uplift a patch that alerts Fennec users if they have an incorrect APK installed. Could you please nominate the patch for uplift to Beta44? Thanks!

Comment on attachment 8682498 [details] MozReview Request: Bug 1119915 - Show toast if APK API range does not match device. r? Approval Request Comment [Feature/regressing bug #]: No regressing feature/bug, but this has been an increasingly frequent cause of crashes. [User impact if declined]: App crashes on startup without explanation (with this fix, at least there's an explanation). [Describe test coverage new/current, TreeHerder]: No automated tests, verified manually by QA. [Risks and why]: Low-risk, adds check to show error message and finish app if Android version is not supported. [String/UUID change made/needed]: One string.

[Tracking Requested - why for this release]: Increasing crash rate. Release management would like us to uplift this patch to help address the crash rate on beta, but we haven't gotten an okay from l10n yet to uplift this string.

Tracked, this is #3 on FennecAndroid 44.0b2, with product level breakdown on 7 days of data as follows: Product Version Count Percentage Installations FennecAndroid 43.0 4447 56.0% 937 FennecAndroid 44.0b2 1949 24.6% 357

I don't really like the idea of breaking string freeze so late in the cycle: we're only 2 weeks away from Beta sign-offs for l10n, in the middle of holidays and a lot of our locales are bound to work against Aurora. Given the crash numbers I get why we want this landed in Beta. My only request would be to have a patch that hard-code the string: it will be displayed in English for all locales in 44, but at least we won't create unnecessary noise and frustrations.

I made a try push to make sure this actually builds properly.

Comment on attachment 8703205 [details] [diff] [review] Patch for beta (with hard-coded string) Approval Request Comment [Feature/regressing bug #]: No regressing feature/bug, but this has been an increasingly frequent cause of crashes. [User impact if declined]: App crashes on startup without explanation (with this fix, at least there's an explanation). [Describe test coverage new/current, TreeHerder]: No automated tests, verified manually by QA. [Risks and why]: Low-risk, adds check to show error message and finish app if Android version is not supported. [String/UUID change made/needed]: None.

Comment on attachment 8703205 [details] [diff] [review] Patch for beta (with hard-coded string) Despite the string change, I am taking this fix in Beta44 as we are seeing this crash signature increase release over release. With this change in place end-users will be alerted to download the correct APK version for their mobile/tablet device.

Ioana, just wanted to bring this to your attention. If you could validate the fix, that would be awesome. Thanks!

Verified as fixed on 44.0b6, this issue was tested on a Samsung Galaxy S6 Edge with Android 5.1.1, Sony Xperia z2 with 5.0.2 (both API 11 while trying to install API 9 build ). In this case the app was installed and upon tapping on it, a toast is displayed informing the user that the build is not supported. On a gingerbread device, HTC desire S with Android 2.3.3, parsing error is received when tapping the installation button for the API 11 build. Please note that on both API types,when trying to install the x86 build, an installation error is received. (the fix will follow up with https://bugzilla.mozilla.org/show_bug.cgi?id=1222925 )

Ninu, can you check this for 45 too to set verified the flag too? Thanks.

Verified as fixed on the 45.0.2 build on a Sony Xperia Z2 with Android 5.0.2(API 15), HTC Desire HD with Android 2.3.5(API 9) and a ZTE Grand X with Android 4.0.4(X86).