840078 - onerror callback is never triggered over the DOMRequest returned by mozIDOMApplication.launch

Status: RESOLVED FIXED

(Core Graveyard :: DOM: Apps, defect)

Description

[Fernando Jiménez Moreno [:ferjm]]

While writing tests for Bug 821589 I've noticed that the callbacks for the |DOMRequest| returned by |mozIDOMApplication.launch| [1] are never triggered.

[1] https://mxr.mozilla.org/mozilla-central/source/dom/interfaces/apps/nsIDOMApplicationRegistry.idl#86

Comment 1

[Jason Smith [:jsmith]]

Doesn't this basically mean that marketplace (or anyone using the API) won't ever know the success or failure of launching of the app then?

Comment 2

[Fernando Jiménez Moreno [:ferjm]]

Attachment: Fix onerror (v1)

We were missing the "Webapps:Launch:Return:KO" message registration.

Comment 3

[Fernando Jiménez Moreno [:ferjm]]

(In reply to Jason Smith [:jsmith] from comment #1)
> Doesn't this basically mean that marketplace (or anyone using the API) won't
> ever know the success or failure of launching of the app then?

I am afraid so.

Comment 4

[[:fabrice] Fabrice Desré]

Comment on attachment 712459 [details] [diff] [review]
Fix onerror (v1)

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

Note that we never send the "OK" one either because for now we have no way to know if the app launched properly.

Comment 5

[Fernando Jiménez Moreno [:ferjm]]

After talking with Fabrice via IRC, we agreed to only fix the .onerror callback for now.

<ferjm> fabrice, ping
[09:27]  <fabrice> ferjm: pong
[09:27]  <ferjm> fabrice, Hi! regarding Bug 840078
[09:27]  <firebot> Bug https://bugzilla.mozilla.org/show_bug.cgi?id=840078 nor, --, ---, ferjmoreno, NEW, onsuccess/onerror callbacks are never triggered over the DOMRequest returned by mozIDOMApplication.l
[09:27]  * davehunt|mtg is now known as davehunt
[09:27]  <ferjm> do you think it is ok just to trigger the DOMRequest.onsuccess before notifying observers of webapps-launch https://mxr.mozilla.org/mozilla-central/source/dom/apps/src/Webapps.jsm#913 ?
[09:27]  * glob|mtg is now known as glob
[09:27]  <ferjm> fabrice, or should we wait for a potential mozContentEvent notifying about a successful launch?
[09:27]  * ctalbert|afk is now known as ctalbert
[09:27]  <fabrice> ferjm: there's no good solution. If we fire it right away, that just means that the registry sent a signal to open it. But different platforms have very different implementations, and we have no proper glue to get answers back. That will be part of the spring refactoring. Is this blocking your tests?
[09:27]  <ferjm> fabrice, it would be great to have at least the .onerror callback for the tests
[09:27]  <ferjm> fabrice, I don't care that much about the onsuccess one for the tests
[09:27]  <fabrice> ferjm: your patch does that, no?
[09:27]  <ferjm> yes
[09:27]  <fabrice> so let's go with that for now
[09:27]  * mdas|afk is now known as mdas
[09:27]  <ferjm> ok, cool. Thanks!

Comment 6

[Fernando Jiménez Moreno [:ferjm]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/292d5dd1c2a0

Comment 7

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/292d5dd1c2a0

Comment 8

[Fernando Jiménez Moreno [:ferjm]]

Comment on attachment 712459 [details] [diff] [review]
Fix onerror (v1)

[Approval Request Comment]
Bug caused by (feature/regressing bug #): None
User impact if declined: The user won't have feedback and explanations of a failed app launch.
Testing completed: Local testing
Risk to taking this patch (and alternatives if risky): Very low risk
String or UUID changes made by this patch: None

Comment 9

[Lukas Blakk [:lsblakk] use ?needinfo]

Comment on attachment 712459 [details] [diff] [review]
Fix onerror (v1)

Approving for v1-train uplift, low risk and will improve user communications around app install.

Comment 10

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-b2g18/rev/51ed55155038