1241999 - Nightly/Aurora: Show a snackbar to ask for the Storage permission (Updater)

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[Sebastian Kaspari (:sebastian; :pocmo)]

Follow-up from bug  1241887: For Nightly/Aurora builds show a snackbar inside the app if the app does not have the "Storage" permission. Without the permission we cannot update the app.

Comment 1

[Sebastian Kaspari (:sebastian; :pocmo)]

Attachment: MozReview Request: Bug 1241999 - Show snackbar to ask for Storage permission if updater is enabled. r=nalexander

Review commit: https://reviewboard.mozilla.org/r/32253/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/32253/

Comment 2

[Nick Alexander :nalexander [he/him] (PTO Feb 24-March 3)]

Comment on attachment 8711630 [details]
MozReview Request: Bug 1241999 - Show snackbar to ask for Storage permission if updater is enabled. r=nalexander

https://reviewboard.mozilla.org/r/32253/#review28973

wfm, modulo comment nit.

::: mobile/android/base/java/org/mozilla/gecko/BrowserApp.java:761
(Diff revision 1)
> +        if (!AppConstants.RELEASE_BUILD && UpdateServiceHelper.isUpdaterEnabled()) {

Please comment:

The update service is enabled for RELEASE_BUILD, which includes the release and beta channels.  However, no updates are served.  Therefore, we don't trust the update service directly, and try to avoid prompting unnecessarily.  See Bug 1232798.

For your convenience, here's the link to the (poorly titled) ticket where I talk about this with mfinkle: https://bugzilla.mozilla.org/show_bug.cgi?id=1232798

Please get snorp's OK on this too -- he may have details to add.

::: mobile/android/base/java/org/mozilla/gecko/BrowserApp.java:785
(Diff revision 1)
> +        SnackbarHelper.showSnackbarWithAction(this,

Will this get hidden when the user taps away from it?  Is there a way to get it back?  (I think not.)

::: mobile/android/base/strings.xml.in:544
(Diff revision 1)
> +  <string name="updater_permission_allow">&updater_permission_allow;</string>

Hmm, the entity mistakenly landed early.

Comment 3

[Sebastian Kaspari (:sebastian; :pocmo)]

(In reply to Nick Alexander :nalexander from comment #2)
> ::: mobile/android/base/java/org/mozilla/gecko/BrowserApp.java:785
> (Diff revision 1)
> > +        SnackbarHelper.showSnackbarWithAction(this,
> 
> Will this get hidden when the user taps away from it?  Is there a way to get
> it back?  (I think not.)

Yeah, we dismiss the snackbar as soon as the user touches anything else. This snackbar will come back after a complete fresh start of the app. And if there's actually an update waiting then we'll show a notification (bug 1241887). I think that's the best compromise: The user sees it but it's not over-annoying.

(In reply to Nick Alexander :nalexander from comment #2)
> ::: mobile/android/base/strings.xml.in:544
> (Diff revision 1)
> > +  <string name="updater_permission_allow">&updater_permission_allow;</string>
> 
> Hmm, the entity mistakenly landed early.

I actually did this on purpose when I landed the notification patch (bug 1241887) so that we can uplift this (Antlam approved all strings).

Comment 4

[Sebastian Kaspari (:sebastian; :pocmo)]

Comment on attachment 8711630 [details]
MozReview Request: Bug 1241999 - Show snackbar to ask for Storage permission if updater is enabled. r=nalexander

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/32253/diff/1-2/

Comment 5

[Sebastian Kaspari (:sebastian; :pocmo)]

Comment on attachment 8711630 [details]
MozReview Request: Bug 1241999 - Show snackbar to ask for Storage permission if updater is enabled. r=nalexander

(In reply to Nick Alexander :nalexander from comment #2)
> Please get snorp's OK on this too -- he may have details to add.

@snorp: This patch should show a snackbar on app start asking for the storage permission in order to download updates (on nightly/aurora only). Do you have any additional feedback or can I land this?

Comment 6

[James Willcox (:snorp) ([e-mail]) (he/him)]

Comment on attachment 8711630 [details]
MozReview Request: Bug 1241999 - Show snackbar to ask for Storage permission if updater is enabled. r=nalexander

I think I would rather have it done in the UpdateService directly, if possible, but this is fine.

Comment 7

[Sebastian Kaspari (:sebastian; :pocmo)]

(In reply to James Willcox (:snorp) ([e-mail]) from comment #6)
> I think I would rather have it done in the UpdateService directly, if
> possible, but this is fine.

This would be nice but we'll need an Activity context to show the snackbar and prompt for the permission. I mean I could try to get that from GeckoAppShell but this isn't nice too.

Comment 8

[Sebastian Kaspari (:sebastian; :pocmo)]

https://hg.mozilla.org/integration/fx-team/rev/f9b4c81ca1a0a8d8a8d82ade1dc6b6e67e009cb4
Bug 1241999 - Show snackbar to ask for Storage permission if updater is enabled. r=nalexander

Comment 9

[Sebastian Kaspari (:sebastian; :pocmo)]

NI self: Consider uplifting this.

Comment 10

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/f9b4c81ca1a0

Comment 11

[Sebastian Kaspari (:sebastian; :pocmo)]

Comment on attachment 8711630 [details]
MozReview Request: Bug 1241999 - Show snackbar to ask for Storage permission if updater is enabled. r=nalexander

Approval Request Comment

[Feature/regressing bug #]: Follow-up from bug 1241887. To download updates for Nightly/Aurora we require the storage permission. We'd like to avoid stranding users because they did not grant the permission.

[User impact if declined]: Since bug 1241887 landed we show a notification asking for the permission if an update is available. We'd like to ask for the permission early (Nightly/Aurora) and inside the app UI. This patch shows a snackbar if the app is started and we do not have the permission. This only affects users of Android 6.0 or higher.

[Describe test coverage new/current, TreeHerder]: Manual testing.

[Risks and why]: Low. Patch is already in Nightly and does only a permission check + showing a snackbar.

[String/UUID change made/needed]: - (Needed string is already on aurora branch)

Comment 12

[Liz Henry (:lizzard) (relman/hg->git project)]

Comment on attachment 8711630 [details]
MozReview Request: Bug 1241999 - Show snackbar to ask for Storage permission if updater is enabled. r=nalexander

Approved for uplift to aurora so that we don't strand users from updates. 
We should verify this on aurora 46 (and please be ready to back it out if it causes regressions)

Comment 13

[Liz Henry (:lizzard) (relman/hg->git project)]

Tracking for 46+, as it is important that we keep updates working for all users.

Comment 14

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/8a188e5f8864

Comment 15

[u421692]

Verified as fixed on latest Nightly on Nexus 9 (Android 6.0)

Comment 16

[Teodora Vermesan (:TeoVermesan)]

Disable storage for Aurora and open it. The following snackbar is displayed:
"To download files and updates, allow Aurora permission to access storage. Allow"
Verified as fixed using:
Device: Nexus 6 (Android 6.0)
Build: Firefox for Android 46.0a2 (2016-02-16)

Comment 17

[Stefan Deiac]

Based on comment 16 I will remove the qe-verify flag, thanks.