New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature request: <webview> app-manifest-updated event #10169

Open
benfrancis opened this Issue Aug 1, 2017 · 7 comments

Comments

Projects
None yet
7 participants
@benfrancis

benfrancis commented Aug 1, 2017

The Electron <webview> has events for notifying its parent when web page metadata changes including:

  • page-title-updated for <title> changes
  • page-favicon-updated for <link rel=icon> changes
  • did-change-theme-color for <meta name=theme-color> changes

I'd like to propose a new event which is dispatched when a web app manifest link relation changes:

  • app-manifest-updated for <link rel=manifest> changes

...as defined by W3C Web App Manifest specification and used by modern progressive web apps.

The payload of the event could simply be the URL of the manifest as with icon link relations, or it could be the manifest JSON content itself with Electron responsible for fetching the manifest in a fully spec compliant way (e.g. using the correct browsing context and supporting HTTP authenticated manifests etc).

For prior art, see the vendor-prefixed mozbrowsermanifestchanged event in Gecko's Browser API (similar to Electron's Webview API).

The use case of this event is to detect when the current web page is part of a web app that can be installed, and use the manifest metadata to install the web app into a web app runtime.

The event name could be different to what I propose here, but note that a web app manifest applies to a whole web app or website which may consist of multiple pages, it it not strictly page metadata in the way that a <title> is.

@benfrancis

This comment has been minimized.

Show comment
Hide comment
@benfrancis

benfrancis Aug 1, 2017

In the meantime, is there a workaround for this? Is there some way to inject a script into the webview context and get it to tell the parent context when a manifest is detected for example?

benfrancis commented Aug 1, 2017

In the meantime, is there a workaround for this? Is there some way to inject a script into the webview context and get it to tell the parent context when a manifest is detected for example?

@alexstrat

This comment has been minimized.

Show comment
Hide comment
@alexstrat

alexstrat Aug 2, 2017

Contributor

@benfrancis regarding a possible workaround, with a similar situation I used MutationObserver in preload script and reported the value back to webContents (or webview) via IPC. It worked for me.

Contributor

alexstrat commented Aug 2, 2017

@benfrancis regarding a possible workaround, with a similar situation I used MutationObserver in preload script and reported the value back to webContents (or webview) via IPC. It worked for me.

@benfrancis

This comment has been minimized.

Show comment
Hide comment
@benfrancis

benfrancis Aug 3, 2017

Thanks @alexstrat I'll give that a try. Presumably a preload script which runs before any of the JavaScript on the page will not be able to access manifest link relations which are inserted into the DOM with JavaScript after the page has loaded. It should work in most cases though.

benfrancis commented Aug 3, 2017

Thanks @alexstrat I'll give that a try. Presumably a preload script which runs before any of the JavaScript on the page will not be able to access manifest link relations which are inserted into the DOM with JavaScript after the page has loaded. It should work in most cases though.

@alexstrat

This comment has been minimized.

Show comment
Hide comment
@alexstrat

alexstrat Aug 3, 2017

Contributor

@benfrancis unless I misunderstood, it can. For instance, you can do something like document.addEventListener("DOMContentLoaded", e => {/* do things */}) in your preload and/* do things */ will be executed at DOMContentLoaded. For your case, I suggest you leverage MutationObserver.

Contributor

alexstrat commented Aug 3, 2017

@benfrancis unless I misunderstood, it can. For instance, you can do something like document.addEventListener("DOMContentLoaded", e => {/* do things */}) in your preload and/* do things */ will be executed at DOMContentLoaded. For your case, I suggest you leverage MutationObserver.

@felixrieseberg

This comment has been minimized.

Show comment
Hide comment
@felixrieseberg

felixrieseberg Aug 3, 2017

Member

Agree with @alexstrat here - since you could enable this behavior in Electron "userland", we should probably not add a method.

Member

felixrieseberg commented Aug 3, 2017

Agree with @alexstrat here - since you could enable this behavior in Electron "userland", we should probably not add a method.

@benfrancis

This comment has been minimized.

Show comment
Hide comment
@benfrancis

benfrancis Aug 4, 2017

I had a play around using a preload script and it seems like that should work in many cases for getting the manifest URL, but I'm not sure I can be fully compliant with the W3C specification for obtaining a manifest. In particular fetching the manifest with the correct "manifest" context, CORS credentials and CSP policy. It seems like fetching and parsing the manifest would really be better handled by the platform.

FWIW I had the same issue when I implemented the W3C web app manifest support in the Firefox browser in Firefox OS. I initially tried to implement it in the app itself but we eventually realised manifest fetching had to be implemented at the platform level to be fully spec compliant.

Agree with @alexstrat here - since you could enable this behavior in Electron "userland", we should probably not add a method.

Not sure I follow your logic. Is that not the case for all of the other examples I gave above (events, not methods) and in fact many features of the Webview API? What decides whether something becomes part of the API vs. requiring users work around it by manually injecting JavaScript into every page? I'd really prefer to avoid injecting JS into pages where possible as it seems a little brittle and doesn't seem like a great idea from a security point of view.

benfrancis commented Aug 4, 2017

I had a play around using a preload script and it seems like that should work in many cases for getting the manifest URL, but I'm not sure I can be fully compliant with the W3C specification for obtaining a manifest. In particular fetching the manifest with the correct "manifest" context, CORS credentials and CSP policy. It seems like fetching and parsing the manifest would really be better handled by the platform.

FWIW I had the same issue when I implemented the W3C web app manifest support in the Firefox browser in Firefox OS. I initially tried to implement it in the app itself but we eventually realised manifest fetching had to be implemented at the platform level to be fully spec compliant.

Agree with @alexstrat here - since you could enable this behavior in Electron "userland", we should probably not add a method.

Not sure I follow your logic. Is that not the case for all of the other examples I gave above (events, not methods) and in fact many features of the Webview API? What decides whether something becomes part of the API vs. requiring users work around it by manually injecting JavaScript into every page? I'd really prefer to avoid injecting JS into pages where possible as it seems a little brittle and doesn't seem like a great idea from a security point of view.

@MarshallOfSound

This comment has been minimized.

Show comment
Hide comment
@MarshallOfSound

MarshallOfSound Aug 4, 2017

Member

I did a quick dive into the Chromium logic that managers the app manifest and it looks like behind the scenes their may be something we can hook in to. But it isn't exposed in as nice a way as other renderer events 😆

https://chromium.googlesource.com/chromium/src.git/+/lkcr/content/renderer/manifest/manifest_manager.h#105

Member

MarshallOfSound commented Aug 4, 2017

I did a quick dive into the Chromium logic that managers the app manifest and it looks like behind the scenes their may be something we can hook in to. But it isn't exposed in as nice a way as other renderer events 😆

https://chromium.googlesource.com/chromium/src.git/+/lkcr/content/renderer/manifest/manifest_manager.h#105

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment