Closed Bug 1265279 Opened 4 years ago Closed 4 years ago

Web Manifest: Implement window.oninstall

Categories

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

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla49
Tracking Status
firefox49 --- fixed

People

(Reporter: marcosc, Assigned: marcosc)

References

(Blocks 1 open bug, )

Details

(Keywords: dev-doc-complete, Whiteboard: btpp-active)

Attachments

(1 file, 3 obsolete files)

Implement the window.oninstall event handler attribute.
Blocks: webmanifest
Whiteboard: btpp-fixlater
Assignee: nobody → mcaceres
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).
Attachment #8753751 - Flags: feedback?(amarchesini)
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?
Attachment #8753751 - Flags: feedback?(bugs)
Attachment #8753751 - Flags: feedback?(amarchesini)
Attachment #8753751 - Flags: feedback+
Attachment #8753751 - Flags: feedback?(bugs) → feedback+
(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.
Attachment #8753751 - Attachment is obsolete: true
Attachment #8755314 - Flags: review?(amarchesini)
... no idea why it worked for me locally... mochitests are gross.
Attachment #8755314 - Attachment is obsolete: true
Attachment #8755314 - Flags: review?(amarchesini)
Attachment #8755683 - Flags: review?(amarchesini)
Whiteboard: btpp-fixlater → btpp-active
Attachment #8755683 - Flags: review?(amarchesini) → review+
Attachment #8755683 - Attachment is obsolete: true
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/9143e6d98cdf
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla49
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.