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

first attempt at XHR spec update #265

Merged
merged 16 commits into from
Jul 13, 2023
Merged

first attempt at XHR spec update #265

merged 16 commits into from
Jul 13, 2023

Conversation

tim-huber
Copy link
Collaborator

@tim-huber tim-huber commented Jul 7, 2023

spec.bs Outdated Show resolved Hide resolved
spec.bs Outdated Show resolved Hide resolved
spec.bs Show resolved Hide resolved
@aykutbulut
Copy link
Collaborator

What I had in my mind was something very similar to what the spec has for fetch. Basically check whether init["privateToken"] exist, if yes, call set private token property for request algorithm.

spec.bs Outdated Show resolved Hide resolved
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.

A couple quick comments. I'll finish reviewing in a bit.

spec.bs Outdated Show resolved Hide resolved
spec.bs Outdated Show resolved Hide resolved
spec.bs Outdated Show resolved Hide resolved
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.

More comments. IM me if you're not sure how to do any of them.

spec.bs Outdated Show resolved Hide resolved
spec.bs Outdated Show resolved Hide resolved
spec.bs Outdated Show resolved Hide resolved
spec.bs Outdated Show resolved Hide resolved
spec.bs Outdated Show resolved Hide resolved
spec.bs Outdated
Integration with XMLHttpRequest {#xhr}
====================================

An XMLHttpRequest has an associated {{PrivateToken}} object specifying the {{OperationType}} to execute against the request.
Copy link
Member

Choose a reason for hiding this comment

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

You need to use this field somewhere to inform the actual request. Is that a monkeypatch to https://xhr.spec.whatwg.org/#the-send()-method to fill in a new Request field?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Added details for send() monkeypatch

spec.bs Outdated Show resolved Hide resolved
tim-huber and others added 7 commits July 11, 2023 10:51
Co-authored-by: Jeffrey Yasskin <jyasskin@gmail.com>
Co-authored-by: Jeffrey Yasskin <jyasskin@gmail.com>
Co-authored-by: Jeffrey Yasskin <jyasskin@gmail.com>
Co-authored-by: Jeffrey Yasskin <jyasskin@gmail.com>
Co-authored-by: Jeffrey Yasskin <jyasskin@gmail.com>
Co-authored-by: Jeffrey Yasskin <jyasskin@gmail.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 after the following changes:

spec.bs Outdated Show resolved Hide resolved
spec.bs Outdated Show resolved Hide resolved
spec.bs Outdated Show resolved Hide resolved
spec.bs Outdated Show resolved Hide resolved
tim-huber and others added 6 commits July 12, 2023 17:19
Co-authored-by: Jeffrey Yasskin <jyasskin@gmail.com>
Co-authored-by: Jeffrey Yasskin <jyasskin@gmail.com>
Co-authored-by: Jeffrey Yasskin <jyasskin@gmail.com>
@tim-huber tim-huber merged commit 1756871 into WICG:main Jul 13, 2023
1 check passed
github-actions bot added a commit that referenced this pull request Jul 13, 2023
SHA: 1756871
Reason: push, by tim-huber

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
github-actions bot added a commit to aykutbulut/trust-token-api that referenced this pull request Jul 13, 2023
SHA: 1756871
Reason: push, by aykutbulut

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
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