Skip to content
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

Reference to opaque origin in the manifest lifecycle #343

Closed
wants to merge 1 commit into from

Conversation

iherman
Copy link
Member

@iherman iherman commented Oct 15, 2018

Adopted the change proposal of @JayPanoz. This replaces the (opaque:-) reference to an opaque origin...

Some issues on security must be addressed in the security section, too, which may, eventually, add some more text to the lifecycle section. But this PR seems to provide a proper intermediate stage.

Fix #321

Cc: @JayPanoz, @rdeltour @llemeurfr @dauwhe


Preview | Diff

@mattgarrish
Copy link
Member

I'm not 100% sold on this, to be honest. Why select only one type of opaque origin to reject? data: URLs, srcdoc, createDocument() all seem like possible ways of generating an opaque-origin document that could attempt to initiate itself as a web publication, as we shouldn't only focus on what is common.

As I read the definition -- and I'm not claiming to be any kind of expert on this -- if a document has an opaque origin, its origin (document.origin) is set to NULL. Isn't that sufficient in terms of determining whether to terminate manifest processing? Only the browser developers really need to understand the finer details of calculating that origin.

@JayPanoz
Copy link

JayPanoz commented Oct 15, 2018

Yeah that was my preferred option as well.

That’s for implementers and It basically takes care of all the details for you, although the concept of “opaque origin” is complex to define/understand at first sight. My assumption is that it will be re-introduced at a later date anyway, because it does that.

Now, I can also understand the need for a temporary clarification, although I’m not comfortable with it – after all, it wasn’t at all in the W3C manifest at first and it could for instance buy some time to write some “companion doc”/wiki explaining how you may end up with an opaque origin and what it means in practice.

@rdeltour
Copy link
Member

I agree with @mattgarrish.

Also, I'm wondering what we're trying to solve with this PR?

  • if the issue is that "opaque" isn't well-defined, then we should open an issue to the HTML standard instead.
  • if the issue is that "opaque" is a concept difficult to comprehend, then I would think that replacing it with "sandboxed" wouldn't make it easier to understand for people not versed in the Web security model. In any case, as Matt correctly points out, this part of the spec is not for laypeople, but for UA developers.

The Web App Manifest issues referenced by @JayPanoz in #321 are on point (especially w3c/manifest#668). I think we should carefully review what happens there, and possibly seek advice from experts (at TPAC?), before jumping into changing anything here.

My 2 cents ;-)

@JayPanoz
Copy link

JayPanoz commented Oct 15, 2018

@rdeltour Note opaque origin impacts media elements as well, audio in particular. But I can’t tell what’s happening there if the media element is CORS cross-origin.

Edit: so yeah restricted, cf. https://stackoverflow.com/a/27431853

@iherman
Copy link
Member Author

iherman commented Oct 15, 2018

O.k. I am fine closing this PR without merge, but I would also think that the comment of @mattgarrish (ie, #343 (comment)) also holds, as an answer, to #321, which may have to be closed without further action, too (although it was very useful to have it, because it helped to clarify certain things and accept our own limitations:-)

Note that, as far as I understand, w3c/manifest#668 is not fully relevant for us either. The manifest lifecycle in this draft is simplified, because there is no mention, in the case of the Web Manifest, of start_url, of the url where the application is to be installed, etc. In this respect, we are on a much simpler space.

@JayPanoz
Copy link

but I would also think that the comment of @mattgarrish (ie, #343 (comment)) also holds, as an answer, to #321, which may have to be closed without further action, too

LGTM.

(I did really think a lot about it recently but my view hasn’t change that much: I personally consider it a whole i.e. the Web Origin + its security policies so it’s probably more of an overarching concept.)

@iherman
Copy link
Member Author

iherman commented Oct 16, 2018

Closing the PR without further action.

@iherman iherman closed this Oct 16, 2018
@mattgarrish mattgarrish deleted the opaque-origin-issue-321 branch October 17, 2018 17:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants