Adding file_handlers and launch consumer #1005
Conversation
|
@mkruisselbrink and @inexorabletash can you PTAL for structural & spec-language feedback? |
Co-authored-by: Thomas Steiner <[email protected]>
|
Alright I think I have it kind of all in place? The execution algorithm isn't good yet - but I think this is the right structure? @inexorabletash PTAL again? @evanstade PTAL? |
index.html
Outdated
| </li> | ||
| </ol> | ||
| <p> | ||
| On [=installation=], a user agent SHOULD register the file handlers |
There was a problem hiding this comment.
I'm not sure what the technical definition of SHOULD is. For example, on Linux some of these things don't happen, i.e. we don't intend to support iconography on Linux. Does that fit within the definition of SHOULD?
There was a problem hiding this comment.
I think the "that are consistent with" here provides some wiggle room based on the OS. I'm not an expert in this though.
There was a problem hiding this comment.
I believe this language was suggested by @inexorabletash
|
@marcoscaceres do you think you could give this a review? I'm currently not sure about:
Do you have some cycles to take a look? Thanks |
dmurph
changed the title
index.html
Outdated
| </p> | ||
| <p> | ||
| When a [=file type=] is registered with a web app and one or more of | ||
| these registered files are launched on the platform, run the following |
There was a problem hiding this comment.
these steps are also run after every same-origin page navigation (this supports refreshing the page or redirecting)
There was a problem hiding this comment.
Hm.... @inexorabletash or @marcoscaceres I need some spec help here - how do you think I should specify this? do the above "assigned launch consumer" and "unconsumed launch params" live across same-origin redirects?
There was a problem hiding this comment.
I think browsing session is where you'll need to have the launch params live (rather than window), and then a window has access to the params via its browsing session.
I have low confidence in my answer here, maybe @jakearchibald can share some wisdom?
There was a problem hiding this comment.
these steps are also run after every same-origin page navigation
Can you explain that in a bit more detail? Let's say I launch //origin1/foo/ using a file, then I click a link in the resulting page to //origin1/bar/ (creating a same-origin navigation), are you saying the next page gets the file handler too? Or is it just to handle reloads?
Either way, it seems like this data needs to go on the session history entry in some way.
There was a problem hiding this comment.
I believe it should just be for redirects and not navigations? But I'm not sure. @evanstade probably has more info here.
There was a problem hiding this comment.
After some more thought, I think this makes more sense if it's based on the app scope. That is, any navigation or reload that remains within app scope gets the file handles. (Originally I was thinking of origin because it's the standard security boundary, but if we're supporting scope extensions then I suppose the security boundary ought to be a set of origins.)
There was a problem hiding this comment.
This section has overlap with this proposal: https://github.com/WICG/sw-launch/blob/main/launch_handler.md#launchqueue-and-launchparams-interfaces
@alancutter ptal
There was a problem hiding this comment.
Thanks for the @, this proposed "re enqueue LaunchParams on in scope navigation" is much less ideal with Launch Handler's proposed behaviour.
Whenever a web app is launched (via any launch trigger) a
LaunchParamsobject will be enqueued in thelaunchQueueglobalLaunchQueueinstance for the browsing context that handled the launch.
If it's not possible to distinguish redirects from user navigations then we should give the act of forwarding LaunchParams to redirect URLs an explicit API e.g.
launchQueue.setConsumer(params => {
if (needsFancyEditor(params.files)) {
params.redirectTo(fancyEditorUrl); // Must be in web app scope.
}
});There was a problem hiding this comment.
@jakearchibald do you have thoughts?
@alancutter does the existing spec here generally work for you other than the navigation scope part?
There was a problem hiding this comment.
I wrote a small doc for removing the re-enqueuing on navigation implementation currently in Chrome.
The LaunchQueue/LaunchParams API surface LGTM otherwise.
index.html
Outdated
| </p> | ||
| <p> | ||
| When a [=file type=] is registered with a web app and one or more of | ||
| these registered files are launched on the platform, run the following |
There was a problem hiding this comment.
Hm.... @inexorabletash or @marcoscaceres I need some spec help here - how do you think I should specify this? do the above "assigned launch consumer" and "unconsumed launch params" live across same-origin redirects?
Add launch queue and launch params section
…to dmurph-file-handling
Add algorithm for matching file handler item.
|
Hi @marcoscaceres and @jakearchibald --- the PR has been updated and the sections on launching a web app with file handles is now also ready for review. Our intent is to leave the launchqueue/launchparams section in this spec for now and move it into a different doc while preparing the spec changes for |
index.html
Outdated
| </h3> | ||
| <p> | ||
| The [=manifest's=] <code><dfn data-export="" data-dfn-for= | ||
| "manifest">`file_handlers`</dfn></code> member is a [=list=] that |
There was a problem hiding this comment.
| "manifest">`file_handlers`</dfn></code> member is a [=list=] that | |
| "manifest">file_handlers</dfn></code> member is a [=list=] that |
index.html
Outdated
| that originate outside of the app itself. | ||
| </p> | ||
| <p> | ||
| The management, presentation and selection of registered |
There was a problem hiding this comment.
| The management, presentation and selection of registered | |
| The management, presentation, and selection of registered |
index.html
Outdated
| </ul> | ||
| <p> | ||
| A user agent MAY truncate the total set of [=file extensions=] to | ||
| preserve functionality and prevent abuse. |
There was a problem hiding this comment.
I wonder if we should explicitly ban some types (e.g., .sys, .exe, etc.)?
There was a problem hiding this comment.
actually, it would probably be enough to say that the UA may prevent certain file associations...
index.html
Outdated
| extension=]. A <dfn>file extension</dfn> is a string that start with a | ||
| "." and only contains [=valid suffix code points=]. A <dfn>valid suffix | ||
| code point</dfn> is a [=code point=] that is [=ASCII alphanumeric=], | ||
| U+002B (+), or U+002E (.). Additionally, [=file extensions=] are are |
There was a problem hiding this comment.
| U+002B (+), or U+002E (.). Additionally, [=file extensions=] are are | |
| U+002B (+), or U+002E (.). Additionally, [=file extensions=] are |
index.html
Outdated
| </p> | ||
| <aside class="note"> | ||
| <p> | ||
| Note: These code points were chosen to support most pre-existing file |
There was a problem hiding this comment.
| Note: These code points were chosen to support most pre-existing file | |
| These code points were chosen to support most pre-existing file |
index.html
Outdated
| </p> | ||
| </aside> | ||
| <aside class="note"> | ||
| <p> |
There was a problem hiding this comment.
This note sounds somewhat normative....
There was a problem hiding this comment.
I've deleted this text because I believe the idea of it is covered by the normative text under #registering-file-handlers
index.html
Outdated
| <p> | ||
| The [=file handler=]'s <code><dfn data-dfn-for= | ||
| "file handler">action</dfn></code> member is a <a>string</a> that | ||
| represents a relative URL of the [=manifest/start_url=] origin that |
There was a problem hiding this comment.
This should probably be resolved relative to the manifest's URL instead (like other URLs in the manifest).
| </h3> | ||
| <p> | ||
| The [=file handler=]'s <code><dfn data-dfn-for= | ||
| "file handler">name</dfn></code> member is a <a>string</a> that |
There was a problem hiding this comment.
I think this should work:
| "file handler">name</dfn></code> member is a <a>string</a> that | |
| "manifest/file_handler">name</dfn></code> member is a <a>string</a> that |
There was a problem hiding this comment.
This suggestion errors to be displayed whereas the current format works as expected. Is there a problem with the current format?
index.html
Outdated
| <p> | ||
| The [=file handler=]'s <code><dfn data-dfn-for= | ||
| "file handler">name</dfn></code> member is a <a>string</a> that | ||
| describes the file type. User agents MAY pass this information to the |
There was a problem hiding this comment.
Something doesn't seem right here (might just be the wording)... it's a "name" that "describes".
Also, I'm a bit unsure where this gets shown to a user? Is there potential abuse here? Like "name": "Pictures" and "type": "application/whatever".
There was a problem hiding this comment.
Something doesn't seem right here (might just be the wording)... it's a "name" that "describes".
Being descriptive is a property of a good name, I think :) But I shall change this to "identifies".
Also, I'm a bit unsure where this gets shown to a user?
The non-normative note right below describes where it's shown to the user. Do you have suggestions on how to make that text more clear?
Is there potential abuse here? Like "name": "Pictures" and "type": "application/whatever"
I can't see a security threat here, but then it's not my specialty. We have, however, subjected this API to several rounds of internal security review and while spoofing in general is a keen concern, the name is not as much of a focus as the icon. I left the wording here more relaxed ("MAY") to accommodate that, depending on the level of trust the user agent places in the app. For example, a user agent with a lot of ceremony around installation may trust installed apps enough to display the name and icon, whereas a user agent where installation is totally frictionless may not want to display name and icon at all.
index.html
Outdated
| The [=file handler/name=] is used to describe the file type in a | ||
| human readable format, and is typically only displayed in OS surfaces | ||
| (such as a file browser) if the web app has become the default | ||
| handler for that file type. It's most useful for custom file types. |
There was a problem hiding this comment.
| The [=file handler/name=] is used to describe the file type in a | |
| human readable format, and is typically only displayed in OS surfaces | |
| (such as a file browser) if the web app has become the default | |
| handler for that file type. It's most useful for custom file types. | |
| <p> | |
| The [=file handler/name=] is used to describe the file type in a | |
| human readable format, and is typically only displayed in OS surfaces | |
| (such as a file browser) if the web app has become the default | |
| handler for that file type. It's most useful for custom file types. | |
| </p> |
index.html
Outdated
| "multiple-clients". If not provided, the default behavior matches | ||
| "single-client". |
There was a problem hiding this comment.
| "multiple-clients". If not provided, the default behavior matches | |
| "single-client". | |
| "multiple-clients". If not provided, it defaults to | |
| "single-client". |
There was a problem hiding this comment.
multiple-clients and single-client should probably link to their definitions and behavior.
There was a problem hiding this comment.
Which thing should link to which thing, specifically?
The definitions/behavior are in the next paragraph.
| </section> | ||
| <section data-cite="file-system-access permissions"> | ||
| <h2> | ||
| Launch queue and launch params |
There was a problem hiding this comment.
I was actually expecting this to be an event, similar to drag and drop. Like, the app starts, and then as the files are read from disk async, events fire at the page.
There was a problem hiding this comment.
This queue helps avoid races between app readiness and file handle readiness. If you tried to drag and drop a file into an app window milliseconds after launching it, it might very well do nothing.
@alancutter as owner of this API
There was a problem hiding this comment.
By having a persistent launchQueue we avoid racing with JS event registration during page load.
index.html
Outdated
| that originate outside of the app itself. | ||
| </p> | ||
| <p> | ||
| The management, presentation and selection of registered |
index.html
Outdated
| </h3> | ||
| <p> | ||
| The [=manifest's=] <code><dfn data-export="" data-dfn-for= | ||
| "manifest">`file_handlers`</dfn></code> member is a [=list=] that |
| </p> | ||
| <section> | ||
| <h3> | ||
| Registering file handlers |
There was a problem hiding this comment.
I don't think there's any file type that we wouldn't want to allow access to. Access control should be applied to individual files that a user shouldn't be able to edit, as you say, by the OS. If a user is allowed to edit a file, I think there's a business justification for wanting to do it via a PWA. If we block specific file types from being edited, we run the risk of forcing apps/users to other ecosystems which don't have this restriction and are much less secure overall. And the greatest threat to user security/privacy is probably in the divulgence of PII or other secrets, which are more likely to be in .doc or .txt or .xlsx or .pdf than some obscure type --- these are clearly types we need to support.
index.html
Outdated
| </ul> | ||
| <p> | ||
| A user agent MAY truncate the total set of [=file extensions=] to | ||
| preserve functionality and prevent abuse. |
index.html
Outdated
| extension=]. A <dfn>file extension</dfn> is a string that start with a | ||
| "." and only contains [=valid suffix code points=]. A <dfn>valid suffix | ||
| code point</dfn> is a [=code point=] that is [=ASCII alphanumeric=], | ||
| U+002B (+), or U+002E (.). Additionally, [=file extensions=] are are |
index.html
Outdated
| <p> | ||
| The [=file handler=]'s <code><dfn data-dfn-for= | ||
| "file handler">action</dfn></code> member is a <a>string</a> that | ||
| represents a relative URL of the [=manifest/start_url=] origin that |
index.html
Outdated
| "multiple-clients". If not provided, the default behavior matches | ||
| "single-client". |
There was a problem hiding this comment.
Which thing should link to which thing, specifically?
The definitions/behavior are in the next paragraph.
| </section> | ||
| <section data-cite="file-system-access permissions"> | ||
| <h2> | ||
| Launch queue and launch params |
There was a problem hiding this comment.
This queue helps avoid races between app readiness and file handle readiness. If you tried to drag and drop a file into an app window milliseconds after launching it, it might very well do nothing.
@alancutter as owner of this API
index.html
Outdated
| The [=file handler/name=] is used to describe the file type in a | ||
| human readable format, and is typically only displayed in OS surfaces | ||
| (such as a file browser) if the web app has become the default | ||
| handler for that file type. It's most useful for custom file types. |
| </h3> | ||
| <p> | ||
| The [=file handler=]'s <code><dfn data-dfn-for= | ||
| "file handler">name</dfn></code> member is a <a>string</a> that |
There was a problem hiding this comment.
This suggestion errors to be displayed whereas the current format works as expected. Is there a problem with the current format?
|
Hi, |
|
File handlers have been put into WICG in manifest-incubations: |
Closes #626
This change (choose at least one, delete ones that don't apply):
Implementation commitment (delete if not making normative changes):
If change is normative, and it adds or changes a member:
Commit message:
Adds file_handlers member and launch consumer algorithm to handle file launches.
Explainer: https://github.com/WICG/file-handling/blob/main/explainer.md
Person merging, please make sure that commits are squashed with one of the following as a commit message prefix:
Preview | Diff
Preview | Diff