Closed Bug 1075476 Opened 6 years ago Closed 5 years ago

removing download notification restarts Firefox

Categories

(Firefox for Android :: Download Manager, defect)

All
Android
defect
Not set
normal

Tracking

()

RESOLVED FIXED
Firefox 50
Tracking Status
firefox48 --- affected
firefox49 --- affected
fennec 48+ ---
firefox50 --- verified

People

(Reporter: c.ascheberg, Assigned: jonalmeida)

References

Details

(Keywords: reproducible)

Attachments

(3 files, 1 obsolete file)

I am using a Nexus 4, Android 4.4.4.

STR:
1. download a file
2. kill Nightly after download is finished
3. remove download notification

Result:
- Firefox shows up again

Expected:
- Firefox should stay closed
I don't think this is a recent regression. For a while, tapping the download notification did absolutely nothing: bug 997756
tracking-fennec: --- → ?
You don't need a window here. This is probably just from bug 1004495 (but before bug 1004495 we just did nothing in this case).
I flipped us to the new downloads stuff in Firefox 32 (bug 1004495).
tracking-fennec: ? → +
Mentor: wjohnston
Attached patch PatchSplinter Review
This moves our notification handling to a broadcast receiver, at least for the initial handling. If the receiver sees that the notification is being cancelled, and knows that Gecko isn't running, it just throws away the event. 

If Gecko is running, it notifies Gecko (without bringing Fennec to the foreground) so that JS can do things if it wants. I had to pull out some code in NotifcationHelper to build a JSON response to do that.

If its not a cancel notification, we force the intent to be handled by GeckoApp.

I'm going to look into some tests for this code (by mocking out the NotificationManager...)
Attachment #8499199 - Flags: review?(bnicholson)
Comment on attachment 8499199 [details] [diff] [review]
Patch

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

What does Gecko do with the notifications if it's running? Either it's necessary (in which case this patch won't work) or it isn't (in which case dropping it might be better). See comment below.

::: mobile/android/base/NotificationHelper.java
@@ +1,1 @@
> +

Nit: remove newline

@@ +186,5 @@
>          notificationIntent.setData(dataUri);
>          notificationIntent.putExtra(HELPER_NOTIFICATION, true);
>          notificationIntent.putExtra(COOKIE_ATTR, message.optString(COOKIE_ATTR));
> +
> +        // All intents get routed through the notificationReceiver. That lets us bail if we don't want to start Gecko

If the download manager can remove downloads on its own, is it worth notifying Gecko at all? Aside from being explicitly killed, remember that Fennec can also be killed at any point while in the background -- but still alive from the user's perspective -- so it's inconsistent if Gecko sometimes receives notifications and sometimes doesn't. I'd argue that no notifications are better than flaky ones.

::: mobile/android/base/NotificationReceiver.java
@@ +1,1 @@
> +

Nit: remove newline

@@ +15,5 @@
> +import android.net.Uri;
> +import android.util.Log;
> +
> +/* Broadcast receiver for Notifications. Will forward them to GeckoApp (and start Gecko) if they're clicked.
> +   If they're being dismissed will not start Gecko, but may forward them to JS if Gecko is running. */

Nit: use JavaDoc style

/**
 * <comments>
 */
(In reply to Brian Nicholson (:bnicholson) from comment #5)
> If the download manager can remove downloads on its own, is it worth
> notifying Gecko at all? Aside from being explicitly killed, remember that
> Fennec can also be killed at any point while in the background -- but still
> alive from the user's perspective -- so it's inconsistent if Gecko sometimes
> receives notifications and sometimes doesn't. I'd argue that no
> notifications are better than flaky ones.

I don't quite understand this comment. The API having onCancel is a bit of an artifact of the old API, which assumed (and still tries) to kill all notifications when Gecko dies. That obviously doesn't work when we're force killed.

You want to remove that API entirely? We could do that and not switch to a handler at all here I guess.
Comment on attachment 8499199 [details] [diff] [review]
Patch

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

(In reply to Wesley Johnston (:wesj) from comment #6)
> I don't quite understand this comment. The API having onCancel is a bit of
> an artifact of the old API, which assumed (and still tries) to kill all
> notifications when Gecko dies. That obviously doesn't work when we're force
> killed.

OK, so we just need to ping Gecko to make sure that stored notifications are cleaned up so we don't leak them? That's all I was wondering -- just wanted to make sure we aren't relying on this event to do important things that may or may not happen.
Attachment #8499199 - Flags: review?(bnicholson) → review+
Hi,

could you provide a try run, thanks!
Keywords: checkin-needed
filter on [mass-p5]
Priority: -- → P5
Duplicate of this bug: 1086606
(In reply to Carsten Book [:Tomcat] from comment #8)
> Hi,
> 
> could you provide a try run, thanks!
Flags: needinfo?(wjohnston)
Duplicate of this bug: 1089280
Status: NEW → ASSIGNED
In the notification bar select the downloaded picture (file) will first start Firefox, and then start the picture viewer.
I filed bug 1091718 for a way to improve that.
Flags: needinfo?(wjohnston)
Is there any progress in this bug?
This has a patch that never landed. It might be bitrot now. NI Brian since Wes is out.
Flags: needinfo?(bnicholson)
Rebased and pushed to try a few days ago, but it looks like some things have changed: https://treeherder.mozilla.org/#/jobs?repo=try&revision=abb702035a8d. Probably not going to spend any more time on this for now since it's not a high priority.
Flags: needinfo?(bnicholson)
Flags: needinfo?(wjohnston)
Duplicate of this bug: 1160858
FYI, 10+ users have commented on this on Input.  I agree it's not a high priority, but it is being noticed.  https://input.mozilla.org/en-US/?q=clear+%2B+notification+%2B+%28open+reopen+launch%29&date_start=2015-02-11
Duplicate of this bug: 1167388
Re-triage this
tracking-fennec: + → ?
Priority: P5 → --
Duplicate of this bug: 1167755
This might not be high priority, but it causes Firefox to crash (see bug 1167755).
Martyn - Can you pick this up? Chat with Wes and Margaret for background.
Assignee: nobody → mhaigh
tracking-fennec: ? → +
Flags: needinfo?(wjohnston)
This issue is independent of the download manager. When I clear any group of notifications on either of my Android devices (phone or tablet) it will cause Firefox to launch, even if the notifications have nothing to do with Firefox or downloads of any kind.
(In reply to gwaeraurond from comment #27)
> This issue is independent of the download manager. When I clear any group of
> notifications on either of my Android devices (phone or tablet) it will
> cause Firefox to launch, even if the notifications have nothing to do with
> Firefox or downloads of any kind.

Can you present some steps to consistently reproduce your issue?
Because for me, the only notifications that produce the issue are Firefox's...
Flags: needinfo?(gwaeraurond)
(In reply to Nicolae Albu from comment #28)
> for me, the only notifications that produce the issue are
> Firefox's...

For me too.
Renom for new assignee.
Assignee: martyn.haigh+bugzilla → nobody
Status: ASSIGNED → NEW
tracking-fennec: + → ?
This does not reproduce for me on Android M (Nexus 5), the download notification is automatically dismissed when Fennec is killed.
Which might not be the expected behavior either.
Assigned to investigate (which versions does this affect?) and see how we can move this forward. Supposedly, the patch should work, provided it gets rebased.
Assignee: nobody → michael.l.comella
tracking-fennec: ? → +
Actually killing Firefox is not necessary - simply switching back to the home screen or some other app suffices. Swiping away the download notification then causes Firefox to come back into the foreground.

I've tested on Android 4.1.2 (Galaxy S3 Mini) and the download notification persists if Firefox is backgrounded, killed by the low memory killer or even swiped away in the "Recent tasks" list. The notification only disappears if I use the "Force stop" option from Android's app manager.
(In reply to Jan Henning (:JanH) from comment #34)
> Actually killing Firefox is not necessary - simply switching back to the
> home screen or some other app suffices.

Depending on the device, it could be that backgrounding Firefox is getting it killed anyway (which is the same as swiping it away).
Just saw this on Reddit:
https://www.reddit.com/r/firefox/comments/437w9i/firefox_android_notifications/

And found this bug here. I never noticed that behavior but it's indeed weird that Firefox comes up after you dismiss the notification:
https://dxr.mozilla.org/mozilla-central/source/mobile/android/base/java/org/mozilla/gecko/NotificationHelper.java#302-303
tracking-fennec: + → ?
The code sebastian linked to in comment 36 shows we set the deleteIntent to the result of `PendingIntent.getActivity`, which is why Firefox is opening again.

It looks like we're doing this to generically remove our queue of notifications [1]:
124         if (CLEARED_EVENT.equals(notificationType)) {
125             mClearableNotifications.remove(id);
126             // If Gecko isn't running, we throw away events where the notification was cancelled.
127             // i.e. Don't bug the user if they're just closing a bunch of notifications.
128             if (!GeckoThread.isRunning()) {
129                 return;
130             }
131         }

The Activity, since it's a foreground UI Object, is not the place to do this. Maybe we could handle this by receiving the PendingIntent in a BroadcastReceiver/IntentService to update the state. However, the state is kept in memory, which is suspicious in and of itself – I filed bug 1244820.

It looks like mClearableNotifications is used as a list of open notifications that we'd want to close when the Fennec Activity is closed and the deleteIntent is used to clear notifications from the HashMap. It looks like some of this state is syncing information to Gecko, so that could be why this seems unnecessarily complicated.

I can't think of a quick fix off the top of my head, expect storing some of the notification state in memory so we can access it from a Service or BroadcastReceiver (which could handle clearing the mClearableNotifications state).

[1]: https://mxr.mozilla.org/mozilla-central/source/mobile/android/base/java/org/mozilla/gecko/NotificationHelper.java#124
It looks like wes' WIP patch moved the receiving to a BroadcastReceiver which ensures the GeckoThread is started before starting the Activity. The one fallacy here is that the GeckoThread state isn't a perfect mirror of the Activity state and they can diverge more in the future.

That being said, it could be the quick fix we want.
Let's try to get this fixed for 47.
tracking-fennec: ? → 47+
48 maybe?
Mentor: wjohnston2000
tracking-fennec: 47+ → ?
Duplicate of this bug: 1256460
(In reply to Kevin Brosnan [:kbrosnan] from comment #40)
> 48 maybe?

Is there anything users can do to help move this along?
From Mike/Wes's investigation it is a thorny concurrency issue between the Java frontend and the NDK compiled Gecko.
tracking-fennec: ? → 48+
Jonathan was going to take a look into this one.
Assignee: michael.l.comella → jonalmeida942
Re-applied wesj's original patch with minor modifications where GeckoAppShell.createBroadcastEvent has been replaced with GeckoAppShell.notifyObservers.

I've done some simple tests to see if the notification goes way:

1. Open Fennec and download an image.
2. Click the recent apps button and swipe away Fennec.
3. Attempt to swipe away the download notification.
4. Observer that Fennec isn't opened again after swiping.

Is there anything else you think I should try to test out? This does seem like a quick win, but I'm wondering if there is some other behaviour I should try observing for.
Flags: needinfo?(gwaeraurond) → needinfo?(michael.l.comella)
(In reply to Jonathan Almeida (:jonalmeida) from comment #45)
> Is there anything else you think I should try to test out?

Spoke on irc & irl. It'd be good to get context on why we keep the notifications in memory to see what other states could be going on here.

That being said, we think we keep the notifications in memory to sync with Gecko so it might good to also test the state when Gecko is dead but Android is in memory (though, I'm not quite sure how to do that, or if we'd ever hit in the wild).
Flags: needinfo?(michael.l.comella)
(In reply to Jonathan Almeida (:jonalmeida) from comment #45)
> Is there anything else you think I should try to test out? This does seem
> like a quick win, but I'm wondering if there is some other behaviour I
> should try observing for.

Yes, please see bug 1167755.
Even if it doesn't crash (because it has sufficient memory, depending on the specs of the device), it makes Firefox hang a couple of seconds (directly proportional to the number of notifications) after the notifications are cleared during which rendering (and others) is halted - for example, if I have a tab with sufficient vertical scrolling (like a forum or even Bugzilla), if I scroll or zoom, it's jerky and the page remains blurry until all the notifications are "processed".
Comment on attachment 8756553 [details] [diff] [review]
Removing download notification restarts Firefox

According to the comments in the code, we keep the in-memory map of notifications for those that aren't tied to Gecko being alive, i.e. notifications without the 'persistent' flag. We use this map when we call `destroy` in the `NotificationHelper`, which removes only those notifications using the id that we've saved. Now, the destroy call is made only in `GeckoApp.onDestroy` which will only get called when low on resources, and it's also tied to the lifetime of gecko so the notifications would be in-memory along with gecko.

If onDestroy is called, should we be removing all notifications or just the non-persistent ones? That seems to be the main reason to keep the notifications in-memory. So if this is the expected behaviour, then keeping it in-memory is useful.

With the new patch, we check within the BroadcastReceiver if Gecko is running first. If not, we don't send it onward to gecko, otherwise we send it on without checking the in-memory map. But in that situation where gecko isn't there, the map won't be there either, so there won't be anything to keep in sync.
Attachment #8756553 - Flags: review?(michael.l.comella)
Comment on attachment 8756553 [details] [diff] [review]
Removing download notification restarts Firefox

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

This code is hard to follow because of the code duplication. Note that code duplication is generally a red flag – it's inevitable that the two code paths will diverge and most of the time, that's not intentional! Let's get rid of that and I'll have another review pass.

GeckoApp.initialize still calls into NotificationHelper.handleNotificationIntent –  do we still need that call?

Finally, this doesn't apply cleanly for me – be sure to rebase.

::: mobile/android/base/AndroidManifest.xml.in
@@ +239,5 @@
>              </intent-filter>
>          </receiver>
>  
> +        <receiver
> +            android:name="org.mozilla.gecko.NotificationReceiver">

add android:exported="false" - this is *very* important for security reasons.

::: mobile/android/base/java/org/mozilla/gecko/NotificationHelper.java
@@ +52,5 @@
>      private static final String ACTION_ICON_ATTR = "icon";
>      private static final String PERSISTENT_ATTR = "persistent";
>      private static final String HANDLER_ATTR = "handlerKey";
>      private static final String COOKIE_ATTR = "cookie";
> +    public static final String ORIGINAL_COMPONENT_EXTRA = "originalComponent";

nit: Should this be package private too?

@@ +126,5 @@
>                  args.put(ACTION_ID_ATTR, actionName);
>              }
>  
>              Log.i(LOGTAG, "Send " + args.toString());
>              GeckoAppShell.notifyObservers("Notification:Event", args.toString());

This method calls notifyObservers, as does the caller – we don't want to send the event twice, right?

@@ +133,5 @@
>          }
> +        return args;
> +    }
> +
> +    public void handleNotificationIntent(SafeIntent i) {

Why is the code here largely duplicated with the code in NotificationReceiver? Don't we already handle all of the edge cases there?

::: mobile/android/base/java/org/mozilla/gecko/NotificationReceiver.java
@@ +14,5 @@
> +import android.content.Intent;
> +import android.net.Uri;
> +import android.util.Log;
> +
> +/* Broadcast receiver for Notifications. Will forward them to GeckoApp (and start Gecko) if they're clicked.

javadoc is:

/**
 * <code comments here>
 * <line 2>
 */

/** is especially important so we could potentially generate API docs in the future.

@@ +15,5 @@
> +import android.net.Uri;
> +import android.util.Log;
> +
> +/* Broadcast receiver for Notifications. Will forward them to GeckoApp (and start Gecko) if they're clicked.
> +   If they're being dismissed will not start Gecko, but may forward them to JS if Gecko is running. */

If I understand the code correctly, you may want to add that this is the *only* entry point for Notification intents.

nit: "If they're being dismissed, <?> will not start Gecko, but <?>..."

@@ +17,5 @@
> +
> +/* Broadcast receiver for Notifications. Will forward them to GeckoApp (and start Gecko) if they're clicked.
> +   If they're being dismissed will not start Gecko, but may forward them to JS if Gecko is running. */
> +public class NotificationReceiver extends BroadcastReceiver {
> +    private static final String LOGTAG = "NotificationReceiver";

nit: By convention, we prepend the logtag with "Gecko". Also, for a little more safety:

`"Gecko" + NotificationReceiver.class.getSimpleName()`

@@ +42,5 @@
> +            }
> +            return;
> +        }
> +
> +        // Otherwise forward the event to Gecko

nit: Feel free to disagree with me, but one coding strategy I really like is if I'm thinking of writing a comment in-line, I consider abstracting it away to a helper method to help with readability & to make sure comments don't get out-of-date (because method names are less likely to go out-of-date than comments). For example:

public void onReceive(Context context, Intent intent) {
  // early return for errors ...

  if (NotificationHelper.CLEARED_EVENT.equals(notificationType)) {
    maybeNotifyNotificationClearedObservers();
    return;
  }

  forwardMessageToActivity();
}

@@ +43,5 @@
> +            return;
> +        }
> +
> +        // Otherwise forward the event to Gecko
> +        ComponentName name = intent.getExtras().getParcelable(NotificationHelper.ORIGINAL_COMPONENT_EXTRA);

nit: final

::: mobile/android/base/moz.build
@@ +469,5 @@
>      'MotionEventInterceptor.java',
>      'NotificationClient.java',
>      'NotificationHandler.java',
>      'NotificationHelper.java',
> +    'NotificationReceiver.java',

Not something you need to fix but I think it'd be cool to move these Notification* to a new package (like that notifications/ package right below here! :P But there may be a reason they're not in there already).
Attachment #8756553 - Flags: review?(michael.l.comella)
Sorry this took me a long time to finally get back to.

(In reply to Michael Comella (:mcomella) from comment #50)
> Comment on attachment 8756553 [details] [diff] [review]
> Removing download notification restarts Firefox
> 
> Review of attachment 8756553 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> This code is hard to follow because of the code duplication. Note that code
> duplication is generally a red flag – it's inevitable that the two code
> paths will diverge and most of the time, that's not intentional! Let's get
> rid of that and I'll have another review pass.

I'm not sure what I did wrong earlier but the patch I uploaded was merged with an older working patch. It explains why I wasn't seeing any merge conflicts locally but you were. I'll be sure to check carefully when I use `hg bzexport` again.

> GeckoApp.initialize still calls into
> NotificationHelper.handleNotificationIntent –  do we still need that call?

True, I didn't notice that earlier. We don't require it during initialize anymore.

> nit: Feel free to disagree with me, but one coding strategy I really like is
> if I'm thinking of writing a comment in-line, I consider abstracting it away
> to a helper method to help with readability & to make sure comments don't
> get out-of-date (because method names are less likely to go out-of-date than
> comments). For example:
> 
> public void onReceive(Context context, Intent intent) {
>   // early return for errors ...
> 
>   if (NotificationHelper.CLEARED_EVENT.equals(notificationType)) {
>     maybeNotifyNotificationClearedObservers();
>     return;
>   }
> 
>   forwardMessageToActivity();
> }
> 

I like this; using it. It feels a bit alien in our code base, but I'll try to use this style more.

> ::: mobile/android/base/moz.build
> @@ +469,5 @@
> >      'MotionEventInterceptor.java',
> >      'NotificationClient.java',
> >      'NotificationHandler.java',
> >      'NotificationHelper.java',
> > +    'NotificationReceiver.java',
> 
> Not something you need to fix but I think it'd be cool to move these
> Notification* to a new package (like that notifications/ package right below
> here! :P But there may be a reason they're not in there already).

Filed bug 1279301.

Fixed all other nits as well.
MozReview-Commit-ID: 35pn1nntcsV
Attachment #8756553 - Attachment is obsolete: true
Attachment #8761756 - Flags: review?(michael.l.comella)
Margaret, could you take a look at this while I'm on PTO? Thanks!
Flags: needinfo?(margaret.leibovic)
I think Sebastian is better qualified to review this...
Flags: needinfo?(margaret.leibovic) → needinfo?(s.kaspari)
Comment on attachment 8761756 [details] [diff] [review]
Removing download notification restarts Firefox

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

::: mobile/android/base/java/org/mozilla/gecko/NotificationHelper.java
@@ +61,5 @@
>      private static final String CLOSED_EVENT = "notification-closed";
>  
> +    static final String EVENT_TYPE_ATTR = "eventType";
> +    static final String CLEARED_EVENT = "notification-cleared";
> +    static final String ORIGINAL_COMPONENT_EXTRA = "originalComponent";

nit: Why the reordering of EVENT_TYPE_ATTR and CLEARED_EVENT? Seems like previously events and attrs were grouped. The grouping might not be useful - but now it's mixed. :)

@@ -117,5 @@
> -        final String notificationType = data.getQueryParameter(EVENT_TYPE_ATTR);
> -        if (id == null || notificationType == null) {
> -            Log.e(LOGTAG, "handleNotificationEvent: invalid intent parameters");
> -            return;
> -        }

Is this check not needed anymore?

@@ -122,4 @@
>  
> -        // In case the user swiped out the notification, we empty the id set.
> -        if (CLEARED_EVENT.equals(notificationType)) {
> -            mClearableNotifications.remove(id);

We stop removing the notification here but we don't do it in NotificationReceiver now. Is this correct?

@@ +139,5 @@
> +        final Uri data = i.getData();
> +        final String notificationType = data.getQueryParameter(EVENT_TYPE_ATTR);
> +
> +        final JSONObject args = getArgsFromIntent(i);
> +        GeckoAppShell.notifyObservers("Notification:Event", args.toString());

The previous code would not send an event if a JSONException was thrown. Now getArgsFromIntent() will return an empty or partial JSONObject and we will send this event.
Attachment #8761756 - Flags: feedback+
Reflag me for review if needed :)
Flags: needinfo?(s.kaspari)
See Also: → 1281822
> ::: mobile/android/base/java/org/mozilla/gecko/NotificationHelper.java
> @@ +61,5 @@
> >      private static final String CLOSED_EVENT = "notification-closed";
> >  
> > +    static final String EVENT_TYPE_ATTR = "eventType";
> > +    static final String CLEARED_EVENT = "notification-cleared";
> > +    static final String ORIGINAL_COMPONENT_EXTRA = "originalComponent";
> 
> nit: Why the reordering of EVENT_TYPE_ATTR and CLEARED_EVENT? Seems like
> previously events and attrs were grouped. The grouping might not be useful -
> but now it's mixed. :)

The different indentation bothered me, I've put it back but at the end of each group.

> @@ -117,5 @@
> > -        final String notificationType = data.getQueryParameter(EVENT_TYPE_ATTR);
> > -        if (id == null || notificationType == null) {
> > -            Log.e(LOGTAG, "handleNotificationEvent: invalid intent parameters");
> > -            return;
> > -        }
> 
> Is this check not needed anymore?

Thanks for the catch, this should be there again.

> @@ -122,4 @@
> >  
> > -        // In case the user swiped out the notification, we empty the id set.
> > -        if (CLEARED_EVENT.equals(notificationType)) {
> > -            mClearableNotifications.remove(id);
> 
> We stop removing the notification here but we don't do it in
> NotificationReceiver now. Is this correct?

Created bug 1281822 to track removing of mClearableNotifications since it's hard to tell how useful it is.

> @@ +139,5 @@
> > +        final Uri data = i.getData();
> > +        final String notificationType = data.getQueryParameter(EVENT_TYPE_ATTR);
> > +
> > +        final JSONObject args = getArgsFromIntent(i);
> > +        GeckoAppShell.notifyObservers("Notification:Event", args.toString());
> 
> The previous code would not send an event if a JSONException was thrown. Now
> getArgsFromIntent() will return an empty or partial JSONObject and we will
> send this event.

Yes, this seems wrong. I've changed it so that we send the event within the try block itself and avoid returning anything. I've changed the `NotificationReceiver` to do this as well.
Comment on attachment 8764661 [details]
Bug 1075476 - Removing download notification restarts Firefox

https://reviewboard.mozilla.org/r/60384/#review57396
Attachment #8764661 - Flags: review?(s.kaspari) → review+
Pushed by jonalmeida942@gmail.com:
https://hg.mozilla.org/integration/fx-team/rev/3a6ed18fa2a9
Removing download notification restarts Firefox r=sebastian
https://hg.mozilla.org/mozilla-central/rev/3a6ed18fa2a9
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 50
Comment on attachment 8761756 [details] [diff] [review]
Removing download notification restarts Firefox

Thanks Sebastian!
Attachment #8761756 - Flags: review?(michael.l.comella)
Firefox does not open after downloading a file, closing Firefox and removing the download notification, so:
Verified as fixed using:
Device: Nexus 6 (Android 6.0)
Build: Firefox for Android 50.0a1 (2016-06-29)
See Also: → 1320889
You need to log in before you can comment on or make changes to this bug.