Tapping update notification while Fennec isn't running just starts fennec and doesn't offer to download

VERIFIED FIXED

Status

Firefox for Android Graveyard
General
VERIFIED FIXED
8 years ago
6 years ago

People

(Reporter: mossop, Assigned: alexp)

Tracking

Trunk
x86
Mac OS X

Details

Attachments

(2 attachments, 5 obsolete attachments)

(Reporter)

Description

8 years ago
Apparently it is possible for Fennec to detect an update and show the update available notification and then close down in the background leaving the notification around. The notification says to tap it to start downloading but when you do Fennec gets launched and nothing else happens
Assignee: nobody → alexp
(Reporter)

Updated

7 years ago
tracking-fennec: --- → ?
tracking-fennec: ? → 2.0b3+
Mossop, do you still see this?
(Assignee)

Comment 2

7 years ago
This is actually how notifications are currently implemented. They may be shown in the status bar regardless of Fennec being running. But if Fennec closed since it displayed the notification, the listener provided to the nsIAlertsService.showAlertNotification obviously gets lost, so when you click on the notification, Fennec activity starts, but there is no listener registered.
I don't think we can restore the listener on restart, so might want just to hide the notification when Fennec closes, and have it to re-check the update status on restart (which actually should happen automatically after the specified interval).
(Assignee)

Comment 3

7 years ago
(In reply to comment #2)
> ...might want just to hide the notification when Fennec closes...

I meant when it gets closed completely by the system, if we can detect that at all.
(In reply to comment #2)

> I don't think we can restore the listener on restart, so might want just to
> hide the notification when Fennec closes, and have it to re-check the update
> status on restart (which actually should happen automatically after the
> specified interval).

This sounds like a good plan to me.

We can use the "xpcom-shutdown" or "quit-application" notifications
(Reporter)

Comment 5

7 years ago
Can we have it flag the update at the next restart or something in that case?
An assumption: I don't think people on android have any real sense of whether a given app is open or not.  Partly as a result of this, I suspect that people think of notifications as being more a system thing than an app thing.

So, in my perfect world, if there's an update there, you could just make it do it's thing, regardless of whether Firefox is open or not.  I guess this will be situation for our updates once we're in the Market, correct?

Definitely, though, if it's going to result in a dead-end, better to not show it.

If you've downloaded it, so the notification you have is that you're ready to install, can that go ahead even if Firefox is closed?
(Assignee)

Comment 7

7 years ago
It looks like we cannot do anything on shut down. Most of the time Fennec gets killed without onDestroy() being called.

It is possible to do something special when Fennec restarts on the notification click, but the problem here is that we don't want to do a lot of custom stuff in Java layer. Normally notifications just call the listener provided, so the notification handler code is pretty generic - it doesn't know if the notification was about update, or file download, or add-on installation. We may think about providing some special command-line parameters, which would let Fennec know on loading that it was started from the notification, and needs to do something about it.
(Assignee)

Comment 8

7 years ago
(In reply to comment #6)
> So, in my perfect world, if there's an update there, you could just make it do
> it's thing, regardless of whether Firefox is open or not.  I guess this will be
> situation for our updates once we're in the Market, correct?

Yes, with Market-based update it will work like this. But updates for nightly/beta builds are performed by our own code, so the app needs to run at that moment.

> If you've downloaded it, so the notification you have is that you're ready to
> install, can that go ahead even if Firefox is closed?

Yes, if the update was downloaded and is ready to install, the installation will happen on the next start of Fennec. So the issue is just with the first notification about the update being ready for download.

Actually regular file downloads have the same issue - if Fennec was shut down, clicking on the notification will restart it, but will not open the download manager. It's less noticeable though, as when something is downloading, the browser is usually active at that moment.
Can we use the same solution as bug 605173?
(Assignee)

Comment 10

7 years ago
(In reply to comment #9)
> Can we use the same solution as bug 605173?

I'll just copy that comment here where it belongs:
==================================================
Mark Finkle (:mfinkle) 2010-11-04 10:53:39 PDT

Can we get the Update notification to launch Fennec with a commandline param? 

fennec -alert <cookie>

so:
fennec -alert update

Then the commandline handler can run some code
==================================================

Yes, I think we can do this, have a generic alert command line parameter handler, which will receive the alert name+cookie, and let chrome decide what to do with that.
(Reporter)

Updated

7 years ago
Duplicate of this bug: 609754
general consensus is to go with the command line flag approach. Let's get a patch together for that and see how it works.
tracking-fennec: 2.0b3+ → 2.0+
(Assignee)

Comment 13

7 years ago
Created attachment 497026 [details] [diff] [review]
[WIP] Fix

Pass alert name and cookie as an -alert command line parameter.
(Assignee)

Comment 14

7 years ago
Created attachment 497028 [details] [diff] [review]
[WIP] Fix (mobile)

Check "-alert" command line flag in command line handler.
This is just a draft, which needs to be reworked. The issue with this approach is that update cannot proceed when called from there - it seems like this command line processing is happening at so early stages that some important modules are not yet initialized.
An attempt to do an update at that moment fails with an error "transfer error: Network is offline (go online), code: 2152398918". We probably need to postpone the actual handling of this command line parameter to some later stage. This needs further investigation.
(Assignee)

Comment 15

7 years ago
Created attachment 498782 [details] [diff] [review]
[WIP] Fix (mobile)

This command line handler now works with updates.
The only issue to be ironed out is with other notifications, like file downloads and addons. With the current implementation the download or addon panel shows empty, and needs to be manually refreshed to show the content.
Attachment #497028 - Attachment is obsolete: true
(Assignee)

Comment 16

7 years ago
Created attachment 499467 [details] [diff] [review]
Fix

Updated to the latest code.
Attachment #497026 - Attachment is obsolete: true
Attachment #499467 - Flags: review?(blassey.bugs)
(Assignee)

Comment 17

7 years ago
Created attachment 499468 [details] [diff] [review]
Fix (mobile)

This version works with update, download and add-on notifications.

There was a concern about "UIReadyDelayed" event listener being in race condition with the browser panels initialization, but it seems to be fine. The listener in BrowserCLH is registered after the one in BrowserUI, so it's called after the downloads/addons panels have been initialized.
Attachment #498782 - Attachment is obsolete: true
Attachment #499468 - Flags: review?(mark.finkle)
Attachment #499467 - Flags: review?(blassey.bugs) → review+
Comment on attachment 499468 [details] [diff] [review]
Fix (mobile)

You'll need to remove this:
http://mxr.mozilla.org/mobile-browser/source/components/BrowserCLH.js#209

I added it in the hopes that it would speed up Ts, but it didn't. Now, it will return early and skip your new code.

>     // First, get a browserDOMWindow object
>     while (!win.browserDOMWindow)
>       Services.tm.currentThread.processNextEvent(true);
> 
>     // Open any URIs into new tabs
>     for (let i = 0; i < uris.length; i++)
>       win.browserDOMWindow.openURI(uris[i], null, Ci.nsIBrowserDOMWindow.OPEN_NEWTAB,
>                                    Ci.nsIBrowserDOMWindow.OPEN_EXTERNAL);

>+      } else if (alertFlag == "addons") {
>+        showPanelWhenReady(win, "addons-container");
>+      }
>+    }
>+
>   },

Don't add the blank line
Attachment #499468 - Flags: review?(mark.finkle) → review+
(Assignee)

Comment 19

7 years ago
Created attachment 502156 [details] [diff] [review]
Fix

Updated to the latest code. r=blassey
Attachment #499467 - Attachment is obsolete: true
Attachment #502156 - Flags: review+
(Assignee)

Comment 20

7 years ago
Created attachment 502157 [details] [diff] [review]
Fix (mobile)

Fixed review comments. r=mfinkle
Attachment #499468 - Attachment is obsolete: true
Attachment #502157 - Flags: review+
pushed:
http://hg.mozilla.org/mobile-browser/rev/0c7acbb43cef
http://hg.mozilla.org/mozilla-central/rev/8f1c39add00f
Status: NEW → RESOLVED
Last Resolved: 7 years ago
Resolution: --- → FIXED
Verified fixed, using:
Mozilla/5.0 (Android; Linux armv7l; rv:2.0b9pre) Gecko/20110109 Firefox/4.0b9pre Fennec/4.0b4pre ID:20110109042046

Steps I used to verify:
- go to about:firefox, tap on "Check for Updates"
- Update available notification appears in Android status bar
- Close Fennec by tapping on the close button of the last open tab
- Open Android status bar and tap on the Fennec update available entry

Expected result:
- Fennec starts up and shows the Update prompt

The expected result is what happens in the build that I used.
Status: RESOLVED → VERIFIED

Comment 23

6 years ago
I'm seeing this again on Fennec Native.
You need to log in before you can comment on or make changes to this bug.