Marina Samuel [:emtwo] Reporter
	Description • 11 years ago

      No description provided.

	Sam Foster [:sfoster] (he/him) Assignee
	Comment 1 • 11 years ago

I'll take this. I'm assuming parity with desktop's behavior wrt. disconnections and crashes is the goal and this bug is mostly about getting the UI in the right state. So, p=3

	Sam Foster [:sfoster] (he/him) Assignee
	Comment 2 • 11 years ago

The profile changes necessary to allow auto-discovery and install of extensions in your profile.

	Sam Foster [:sfoster] (he/him) Assignee
	Comment 3 • 11 years ago

As a prerequisite for this ticket (downloads handling after a crash), this zip is a version of the crashme extension with Metro hacked in. It adds the Metro appid to the install.rdf, and a Settings -> Crash Now! entry which should crash the browser. 

If this looks useful, I can iterate to get the changes into the extension for real on http://code.google.com/p/crashme/ (no pull requests on google code - sadface)

	Sam Foster [:sfoster] (he/him) Assignee
	Comment 4 • 11 years ago

Patch with just the changes to install.rdf and bootstrap.js.

	(not currently active) Ted Mielczarek
	Comment 5 • 11 years ago

Comment on attachment 793869 [details] [diff] [review]
Modified crashme extension to add Metro support

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

Looks sane. If this works for you then that's fine with me. I'll land it in the Google Code repo and put up a repacked version of the extension on my people account. (Sorry about Google Code, but it gets so few commits that it's less hassle than moving it to github.)

	Sam Foster [:sfoster] (he/him) Assignee
	Comment 6 • 11 years ago

Ok so I tested out the .xpi and it looks good and works well. I'll be updating our wiki with instructions on how to install and use it, so let me know when its up on google code.

	(not currently active) Ted Mielczarek
	Comment 7 • 11 years ago

The best install link to use is here, FYI:
http://people.mozilla.com/~tmielczarek/crashme/

	(not currently active) Ted Mielczarek
	Comment 8 • 11 years ago

I pushed your change to SVN as well:
http://code.google.com/p/crashme/source/detail?r=19

	Sam Foster [:sfoster] (he/him) Assignee
	Comment 9 • 11 years ago

This is where I'm at. I've refactored a little and I've got any resuming downloads being correctly handled at startup. But it looks like the notifications are being hidden immediately at startup? As far as I can tell from the logging, all the right things are happenning. And sometimes you get a brief flash of what might be the notification bar disappearing as the rest of the UI displays. Ideas?

	Marina Samuel [:emtwo] Reporter
	Comment 10 • 11 years ago

Am wondering if this is related to bug 906772. If notifications are re-triggered at startup while the navbar is visible, they may be hiding behind the navbar. Though I still haven't looked at this patch, this is just an idea.

	Sam Foster [:sfoster] (he/him) Assignee
	Comment 11 • 11 years ago

Yeah I wondered the same. I've got logging in CAO now and I see that notifications are *not* hidden at startup when there's a resumed download, but allNotifications.length == 0. So I'm still missing something when creating those notifications. It so almost works :)

	Sam Foster [:sfoster] (he/him) Assignee
	Comment 12 • 11 years ago

This adds a step to check for active downloads at Downloads.init and notify the user just like we do for "new" downloads. Did a little refactoring to this end. 

Now, if you quit while one or more downloads are in progress, when you next startup you'll get a notification - and an opportunity to cancel resumed downloads. The same happens after a crash. For network disconnects, the download is paused. It will resume when network connectivity is restored. If you quit while disconnected, we'll try to resume when you next startup. 

Note that we don't currently show the download progress button on about:start. So you don't see the progress when you start up and get those notifications. I propose to fix that (in another bug)

	Matt Brubeck (:mbrubeck)
	Comment 13 • 11 years ago

Comment on attachment 801889 [details] [diff] [review]
Notify of resuming downloads at startup

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

::: browser/metro/base/content/downloads.js
@@ +152,5 @@
>    },
>  
>    // Cancels all downloads.
>    cancelDownloads: function dh_cancelDownloads() {
> +    for (let [guid,info] of this._progressNotificationInfo) {

Nit: Add a space after "," please.  (Rationale: guid,info looks too much like guid.info.)

::: browser/metro/components/HelperAppDialog.js
@@ +43,5 @@
>    },
>  
>    _getDownloadSize: function dv__getDownloadSize (aSize) {
>      let displaySize = DownloadUtils.convertByteUnits(aSize);
> +    if (isNaN(displaySize[0]) && displaySize[0] > 0) // [0] is size, [1] is units

should that be "||" instead of "&&"?

	Sam Foster [:sfoster] (he/him) Assignee
	Comment 14 • 11 years ago

Ah, that should have been !isNan() (is a number and is greater than 0). Fixed that and the other nits and landed on fx-team: 
https://hg.mozilla.org/integration/fx-team/rev/23e8350ce32c

	Ryan VanderMeulen [:RyanVM]
	Comment 15 • 11 years ago

https://hg.mozilla.org/mozilla-central/rev/23e8350ce32c