Closed Bug 1241999 Opened 4 years ago Closed 4 years ago

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

Categories

(Firefox for Android :: General, defect)

All
Android
defect
Not set

Tracking

()

VERIFIED FIXED
Firefox 47
Tracking Status
firefox46 + verified
firefox47 + verified

People

(Reporter: sebastian, Assigned: sebastian)

References

Details

(Keywords: regression)

Attachments

(1 file)

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 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.
Attachment #8711630 - Flags: review?(nalexander) → review+
(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 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/
Attachment #8711630 - Attachment description: MozReview Request: Bug 1241999 - Show snackbar to ask for Storage permission if updater is enabled. r?nalexander → MozReview Request: Bug 1241999 - Show snackbar to ask for Storage permission if updater is enabled. r=nalexander
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?
Attachment #8711630 - Flags: feedback?(snorp)
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.
Attachment #8711630 - Flags: feedback?(snorp) → feedback+
(In reply to James Willcox (:snorp) (jwillcox@mozilla.com) 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.
https://hg.mozilla.org/integration/fx-team/rev/f9b4c81ca1a0a8d8a8d82ade1dc6b6e67e009cb4
Bug 1241999 - Show snackbar to ask for Storage permission if updater is enabled. r=nalexander
NI self: Consider uplifting this.
Flags: needinfo?(s.kaspari)
https://hg.mozilla.org/mozilla-central/rev/f9b4c81ca1a0
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 47
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)
Flags: needinfo?(s.kaspari)
Attachment #8711630 - Flags: approval-mozilla-aurora?
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)
Attachment #8711630 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Tracking for 46+, as it is important that we keep updates working for all users.
Flags: qe-verify+
Keywords: regression
Verified as fixed on latest Nightly on Nexus 9 (Android 6.0)
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)
Status: RESOLVED → VERIFIED
Version: unspecified → Trunk
Based on comment 16 I will remove the qe-verify flag, thanks.
Flags: qe-verify+
You need to log in before you can comment on or make changes to this bug.