1265279 - Web Manifest: Implement window.oninstall

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect)

Description

[Marcos Caceres [:marcosc]]

Implement the window.oninstall event handler attribute.

Comment 1

[Marcos Caceres [:marcosc]]

Attachment: 0001-Bug-1265279-Web-Manifest-Implement-window.oninstall.patch

Baku, does this look ok? The event is supposed to be fired by the user agent when a "progressive web app" is successfully installed somewhere (e.g., user's homescreen on a mobile device).

Comment 2

[Andrea Marchesini [:baku]]

Comment on attachment 8753751 [details] [diff] [review]
0001-Bug-1265279-Web-Manifest-Implement-window.oninstall.patch

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

::: dom/ipc/manifestMessages.js
@@ +69,5 @@
> +    const ev = new Event("install");
> +    const response = makeMsgResponse(id);
> +    if (!content || content.top !== content) {
> +      let msg = "Can only dispatch install event on top-level browsing contexts.";
> +      response.result = serializeError(new Error(msg));

should we localize this error message?

Comment 3

[Marcos Caceres [:marcosc]]

(In reply to Andrea Marchesini (:baku) from comment #2)
> Comment on attachment 8753751 [details] [diff] [review]
> 0001-Bug-1265279-Web-Manifest-Implement-window.oninstall.patch
> 
> Review of attachment 8753751 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: dom/ipc/manifestMessages.js
> @@ +69,5 @@
> > +    const ev = new Event("install");
> > +    const response = makeMsgResponse(id);
> > +    if (!content || content.top !== content) {
> > +      let msg = "Can only dispatch install event on top-level browsing contexts.";
> > +      response.result = serializeError(new Error(msg));
> 
> should we localize this error message?

Right now, the error would only be used internally so I don't think it's necessary.

Comment 4

[Marcos Caceres [:marcosc]]

Attachment: Adds tests and little testing server

Comment 5

[Marcos Caceres [:marcosc]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=406558c68631

Comment 6

[Marcos Caceres [:marcosc]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=fed18f455f37

Comment 7

[Marcos Caceres [:marcosc]]

Attachment: Listed support files in browser.ini

... no idea why it worked for me locally... mochitests are gross.

Comment 8

[Marcos Caceres [:marcosc]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=2ff61cd36266

Comment 9

[Marcos Caceres [:marcosc]]

Attachment: Add baku as reviewer.

Comment 10

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/9143e6d98cdf

Comment 11

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/9143e6d98cdf

Comment 12

[Jean-Yves Perrier [:teoli]]

Added an entry in:
https://developer.mozilla.org/en-US/docs/Web/API/Window#Event_handlers

Created:
https://developer.mozilla.org/en-US/docs/Web/API/Window/oninstall
https://developer.mozilla.org/en-US/docs/Web/Events/install

Updated:
https://developer.mozilla.org/en-US/Firefox/Releases/49#Others