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

Update spec.bs to include operations via HTTP response header #110

Merged
merged 12 commits into from
Sep 25, 2023

Conversation

pythagoraskitty
Copy link
Collaborator

@pythagoraskitty pythagoraskitty commented Sep 7, 2023

We update the spec to include how operations can be triggered via HTTP response headers, including by way of fetch() as well as HTML images and iframes.


Preview | Diff

We update the spec to include how operations can be triggered via HTTP response headers, including by way of `fetch()` as well as HTML images and iframes.
Copy link
Collaborator

@xyaoinum xyaoinum left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good % comments. Let's have a spec reviewer to take a look as well.

@pythagoraskitty
Copy link
Collaborator Author

wanderview@, PTAL. Thanks!

@pythagoraskitty pythagoraskitty requested review from jyasskin and removed request for wanderview September 19, 2023 21:25
@pythagoraskitty
Copy link
Collaborator Author

@jyasskin Could you please take a look? I'm trying to get some feedback from a spec expert... thanks

Partially prune the anchors section
Remove 2 blank lines
Fix string and url links
Fix more anchors
change to header type dfn
Split window setter/deleter algos
string algos
Address more comments
Add operation descriptions
@pythagoraskitty
Copy link
Collaborator Author

Landing so that I can open another PR on the spec. Will continue to iterate.

@pythagoraskitty pythagoraskitty merged commit 5c06564 into main Sep 25, 2023
@pythagoraskitty pythagoraskitty deleted the cammie-branch1 branch September 25, 2023 20:06
github-actions bot added a commit that referenced this pull request Sep 25, 2023
SHA: 5c06564
Reason: push, by pythagoraskitty

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Copy link
Member

@jyasskin jyasskin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good. I had a couple minor remaining comments:


This will require monkey patches to the HTML and Fetch specifications.

# HTML Monkey Patches # {#html-monkeypatches}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd suggest making this and the #fetch-monkeypatches section into sub-sections of the #http section. That is, add a # to all the headings you added.


add the step

1. If <var ignore=''>navigable</var>'s [=container=] is an <{iframe}> element, and if it has a <{iframe/sharedstoragewritable}> [=content attribute=], then set |request|'s [=request/shared storage writable=] to true.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You should be able to omit the ='' bit, and just use <var ignore>.

* `key` ([=structured header/Token=], [=structured header/String=], or [=structured header/Byte Sequence=])
* `value` ([=structured header/Token=], [=structured header/String=], or [=structured header/Byte Sequence=])
- If the [=structured header/parameter=] value of `key` already exists in the [=shared storage database=] for the responding server's [=/origin=], then `append` updates the [=entry/key=]'s [=shared storage database/entry=]'s [=entry/value struct=]'s [=value struct/value=] by appending the [=structured header/parameter=] value for `value` to it.
- If the [=structured header/parameter=] value of `key` does not already exist in the [=shared storage database=] for the responding server's [=/origin=], `append` is equivalent to `set` with `ignore_if_present` false, i.e. `append` writes the [=shared storage database/entry=] consisting of the [=structured header/parameter=] values for `key` and `value`, as the [=shared storage database/entry=]'s [=entry/key=] and [=shared storage database/entry=]'s [=entry/value struct=]'s [=value struct/value=] respectively, in the [=shared storage database=] for the responding server's [=/origin=].
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you can omit the "with ignore_if_present false" since it's known not to be present.

1. If |window| is not an [=environment settings object=] whose [=global object=] is a {{Window}}, return false.
1. If the result of running [=determine whether shared storage is allowed=] for |window| is false, return false.
1. Let |document| be |window|'s [=global object=]'s [=associated document=].
1. Return the result of running [=Is feature enabled in document for origin?=] on "[=PermissionsPolicy/shared-storage=]", |document|, and |request|'s [=request/current URL=]'s [=url/origin=].
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should probably use [=Should request be allowed to use feature=]? instead of "Is feature enabled...".

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.

3 participants