777517 - Webapp installation alert should launch app on click

Status: RESOLVED FIXED

(Firefox Graveyard :: Web Apps, defect, P2)

Description

[Dan Walkowski]

The alert displayed when installing a new Webapp is not responsive.
Clicking on it should launch the app.

Comment 1

[Dan Walkowski]

Attachment: enable webapp launch from alert

This patch enables clicking in the Webapp installation alert to launch the app.

Comment 2

[:Felipe Gomes (needinfo for replies!)]

Comment on attachment 645898 [details] [diff] [review]
enable webapp launch from alert

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

Looks good! just some small suggestions

Hopefully we won't even need the forceAppRegistration before landing this, as you told me the ".app" fix might make it unecessary

::: browser/modules/webappsUI.jsm
@@ +145,5 @@
> +                let data = JSON.parse(launchData);
> +                WebappOSUtils.launch(data.app);
> +              }
> +            }
> +          }

you should create this listener inside installationSuccessNotification itself, it does not need to be outside. With that you also won't need to pass the data.app json as the parameter, you can just directly use it from the aData argument in the function.

@@ +150,5 @@
> +
> +
> +function installationSuccessNotification(app, aData, aWindow) {
> +  let mwaUtils = Cc["@mozilla.org/widget/mac-web-app-utils;1"].
> +  createInstance(Ci.nsIMacWebAppUtils);

mwaUtils is not used here, no need to add this part

@@ +168,3 @@
>  
> +    } catch (ex) {
> +      dump(" Notifier Exception: " + ex + "\n");

debugging code

Comment 3

[Marco Castelluccio [:marco]]

Attachment: Patch

Comment 4

[Myk Melez [:myk] [@mykmelez]]

Comment on attachment 782040 [details] [diff] [review]
Patch

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

r=myk with conflict fixed!

::: browser/modules/webappsUI.jsm
@@ +116,5 @@
>  
>            DOMApplicationRegistry.confirmInstall(aData, false, localDir, null,
>              function (aManifest) {
>                if (WebappsInstaller.install(aData, aManifest)) {
> +                installationSuccessNotification(aData, app, aWindow);

This now fails to apply, but the conflict is trivial.

Comment 5

[Marco Castelluccio [:marco]]

Attachment: Patch

Carrying r+

Comment 6

[Matthew N. [:MattN]]

https://hg.mozilla.org/integration/fx-team/rev/3b9baad91485

Comment 7

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/3b9baad91485