856202 - Defect - can't download nightly full installer

Status: VERIFIED FIXED

(Firefox for Metro Graveyard :: General, defect, P1)

Description

[juan becerra [:juanb]]

While testing 850415 I noticed that I couldn't find the nightly full installer after downloading it. After a second look it may not be downloading the file. It certainly isn't listed in the Downloads page.

I tried downloading other files like this video, and that worked: http://www.blueskyfootage.com/Video_Formats/#format3

I also tried some PDF publications from the IRS which open up in the page, but are also downloaded as PDF documents which you can find with the Search charm.

The search charm doesn't list the full installer, and the Downloads folder for the user doesn't have it.

Desktop Firefox has no problem downloading these files.

STR:
1. Go to nightly.mozilla.org
2. Tap on the Windows (Standard) download link
3. Tap on the download bar to start the download
- At this point there should be an item for this installer download, but there isn't any.
4. Swipe to show the Search charm and look for "installer" in the files category.

Expected: You should get a result.

Actual: Nothing found.

You can also try the stub installers or the linux installers, but that also doesn't seem to work.

This might be a dupe of 852014.

Comment 1

[Brian R. Bondy [:bbondy]]

Marco this is pretty close to the work I'm doing in it5 for bug 852014.  Do you mind if I take this during it5 while doing that work?

Comment 2

[Brian R. Bondy [:bbondy]]

> This might be a dupe of 852014.

Agreed it might be, but not completely sure yet until I investigate more.

Comment 3

[Marco Mucci [:MarcoM]]

Hey Brian, no problem.  I'll move Bug 856202 over to Iteration #5.

Comment 4

[Brian R. Bondy [:bbondy]]

Attachment: Patch v1.

What's happening is that nsAlertService.cpp's ShouldShowAlert returns false because we're in immersive mode.  
When ShouldShowAlert returns false it sends out a "alertfinished" event and we cancel the download right away.
See: http://dxr.mozilla.org/mozilla-central/browser/metro/components/HelperAppDialog.js#l44 to see us canceling.
See: http://dxr.mozilla.org/mozilla-central/toolkit/components/alerts/nsAlertsService.cpp#l105 to see us sending the alertfinished because we're in immersive mode.

I first tried to fix this by detecting immersive mode in SHQueryUserNotificationState and returning true in ShouldShowAlert.
But this falls through to "rv = mXULAlerts.ShowAlertNotification" which hangs the browser.

Since we provide our own custom alert service we already call into it here and it succeeds:
http://dxr.mozilla.org/mozilla-central/toolkit/components/alerts/nsAlertsService.cpp#l99
I'm not sure why we don't always return there on all platforms when it succeeds but we have a couple different implementations of that (b2g, linux, possibly more) and I don't want to cause regressions in there.

So to fix this I return on success of our own alert service code.

Comment 5

[Brian R. Bondy [:bbondy]]

This is not a dupe of bug 852014 by the way.
This bug has to do with the popup that comes up when trying to download an installer that asks you to tap to download, and the tap not working (because of auto cancel)

Comment 6

[Jim Mathies [:jimm]]

Is this behavior triggered because it's an executable?

Comment 7

[Brian R. Bondy [:bbondy]]

no not in particular, any type that has no mime default, see the else clause to this condition here:
http://dxr.mozilla.org/mozilla-central/browser/metro/components/HelperAppDialog.js#l28

Comment 8

[Brian R. Bondy [:bbondy]]

To summarize the toolkit code is always firing "alertfinished" right away in Metro and it shouldn't even reach that code in metro.

Comment 9

[Brian R. Bondy [:bbondy]]

and we watch for "alertfinished" and cancel from front end code.

Comment 10

[Jim Mathies [:jimm]]

Comment on attachment 733549 [details] [diff] [review]
Patch v1.

discussed on irc, r+ as a work around. bbondy is going to file a follow up on the code that landed in bug 699014, which doesn't look right.

Comment 11

[Brian R. Bondy [:bbondy]]

***code that landed in bug 782211 caused the regression

Comment 12

[Brian R. Bondy [:bbondy]]

Attachment: Patch v2.

I'm pretty sure this is just a regression introduced in bug 782211 so going ahead with the fix for all platforms. Looks like B2G is the only other one that is affected by this.

Comment 13

[Brian R. Bondy [:bbondy]]

I'll update the patch to r=dougt before pushing after it is reviewed by the way.

Comment 14

[Brian R. Bondy [:bbondy]]

Likely causes problems on b2g since it is the only other platform that implements its own sysAlerts.

Comment 15

[Brian R. Bondy [:bbondy]]

since this is on b2g v22 not sure if you wanted to track or not

Comment 16

[Brian R. Bondy [:bbondy]]

Attachment: Patch v3.

Comment 17

[Brian R. Bondy [:bbondy]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/f49cb7d4b736

Comment 18

[Ed Morley [:emorley]]

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

Comment 19

[raymond [:retornam] (inactive)]

Verified fixed  on April 10th, 2013  built from http://hg.mozilla.org/mozilla-central/rev/9db46ddfb517

The downloaded items are shown on the Downloads tab and can be found using  the search charm.

Comment 20

[Alex Keybl [:akeybl]]

Doug - do you think we need this on m-b2g18? If so, can you mark as blocking-b2g:tef? or approval-mozilla-b2g18:?

Comment 21

[Brian R. Bondy [:bbondy]]

Doug - Basically any kind of alert (based on my reading of the code) *should* show the b2g sysalerts followed by an xul alert.  But I haven't tested this, I'm just going by what I think the code does on b2g.

Comment 22

[Doug Turner (:dougt)]

brian's right -- this should be uplifted everywhere.

Comment 23

[Brian R. Bondy [:bbondy]]

Comment on attachment 734769 [details] [diff] [review]
Patch v3.

[Approval Request Comment]
Bug caused by (feature/regressing bug #): bug 782211 
User impact if declined:  Some B2G notifications might be double prompting or prompting once and then and error happening.
Testing completed (on m-c, etc.): It's been on m-c for a while
Risk to taking this patch (and alternatives if risky): Low
String or IDL/UUID changes made by this patch: None.

Comment 24

[Alex Keybl [:akeybl]]

If https://hg.mozilla.org/releases/mozilla-b2g18/ is impacted, please mark this bug as blocking-b2g:leo?

Comment 25

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/95064e255be8

Comment 26

[Brian R. Bondy [:bbondy]]

(In reply to Alex Keybl [:akeybl] from comment #24)
> If https://hg.mozilla.org/releases/mozilla-b2g18/ is impacted, please mark
> this bug as blocking-b2g:leo?

It doesn't look like it but the bug that introduced this looks like it is tracking b2g18 so it may in the future. You may want to also track this one for it.

Comment 27

[Brian R. Bondy [:bbondy]]

Tracking flag is because the bug that introduced the bug (bug 782211) is marked for tracking b2g18 already.

Comment 28

[Cornel Ionce [:noni] [Hubs QA]]

Verified with 22, 23, 24 and 25 nighty builds.
The full installers are downloaded correctly and shown in the Downloads tab. They can also be found using search charm.