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

Add a prefetch initiator #659

Merged
merged 3 commits into from
Apr 27, 2018
Merged

Conversation

yoavweiss
Copy link
Collaborator

@yoavweiss yoavweiss commented Jan 8, 2018

Closes #658


Preview | Diff

Copy link
Member

@annevk annevk left a comment

Choose a reason for hiding this comment

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

Happy to land this when the CSP PR is ready.

fetch.bs Outdated
@@ -917,6 +919,16 @@ not always relevant and might require different behavior.
<td>"<code>manifest</code>"
<td><code>manifest-src</code>
<td>HTML's <code>&lt;link rel=manifest></code>
<tr>
<td>"<code>prefetch</code>"
<td>""
Copy link
Member

Choose a reason for hiding this comment

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

rowspan=2 for this td and the next? And then remove the from the next row?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

updated. Let me know if this is what you had in mind

Copy link
Member

@annevk annevk left a comment

Choose a reason for hiding this comment

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

I guess the only thing that remains here is Akamai signing up. Thanks for working on this!

<td rowspan=2><code>prefetch-src</code>
<td>HTML's <code>&lt;link rel=prefetch></code>
<tr>
<td>"<code>prerender</code>"
Copy link
Member

Choose a reason for hiding this comment

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

Reading through w3c/webappsec-csp@91adc4a, prefetch-src should also be listed for prerender?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It is (that's what the "rowspan=2" is all about)

Copy link
Member

Choose a reason for hiding this comment

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

Ah, right. Nevermind me, all good!

@igrigorik
Copy link
Member

Looks great, thanks for driving this Yoav!

@yoavweiss
Copy link
Collaborator Author

@annevk - Finally managed to sign the participant agreement on behalf of Akamai. Apologies for the delay. Can you please verify me to appease the bots?

@annevk
Copy link
Member

annevk commented Mar 12, 2018

Done, though I think @domenic needs to flip some switch to make it propagate (I'm happy to learn how btw).

Are the tests for this web-platform-tests/wpt#9013?

Do we have a follow-up issue for dns-prefetch and such that came up in the issue?

@yoavweiss
Copy link
Collaborator Author

Are the tests for this web-platform-tests/wpt#9013?

Yeah. We cannot test the prefetch initiator directly (unlike destination changes), so I guess CSP tests is our best to do that.

Do we have a follow-up issue for dns-prefetch and such that came up in the issue?

I'll try to get to it this week (if I won't, it'll probably be at the end of April, due to travel...)

@annevk
Copy link
Member

annevk commented Mar 12, 2018

Follow-up is #683. I think we should file browser bugs since CSP doesn't have that as a policy. Then this can land.

chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this pull request Apr 26, 2018
Currently `Request.destination` is set to "unknown" prefetch,
but that was recently changed:
Issue: whatwg/fetch#658
PR: whatwg/fetch#659

This CL aligns the destination values to the spec change.

Bug: 832105
Change-Id: Ib9f21dcc6cf0ace27b7a810d3670cddc45b3b74f
@yoavweiss
Copy link
Collaborator Author

@mikewest added a bunch of tests to https://github.com/w3c/web-platform-tests/tree/master/content-security-policy/prefetch-src and I just added tests to make sure the destination is "" at web-platform-tests/wpt@b3312d9

@annevk - is that enough to land this? Or are more tests needed?

@annevk
Copy link
Member

annevk commented Apr 26, 2018

@yoavweiss great, though we already had sufficient tests to land. The one thing lacking here is impl bugs: https://github.com/whatwg/meta/blob/master/MAINTAINERS.md#handling-pull-requests.

@wanderview
Copy link
Member

The initiator is not exposed on Request, correct? So does implementation here mainly mean something for CSP behavior?

@annevk
Copy link
Member

annevk commented Apr 26, 2018

Yes.

@yoavweiss
Copy link
Collaborator Author

Yeah, implementation means implementing prefetch-src directives and enforcing them (as well as an empty destination for prefetch requests)

@yoavweiss
Copy link
Collaborator Author

@wanderview There's already https://bugzilla.mozilla.org/show_bug.cgi?id=1449374 but I'm not sure if it should be converted to an "implement prefetch-src" bug, or marked as a duplicate of one after its creation

@wanderview
Copy link
Member

@yoavweiss I recommend filing a more clear bug about implementing prefetch-src under Core::DOM:Security.

@yoavweiss
Copy link
Collaborator Author

@wanderview on it! :)

@yoavweiss
Copy link
Collaborator Author

@wanderview Filed https://bugzilla.mozilla.org/show_bug.cgi?id=1457204 but don't seem to have permissions to change components

@yoavweiss
Copy link
Collaborator Author

WebKit bug: https://bugs.webkit.org/show_bug.cgi?id=185070 (even though prefetch is not shipped in Safari atm)

Edge bug: I sent out https://twitter.com/yoavweiss/status/989729796895653888 which people say should work. Alternatively @patrickkettner said he'd open one.

@annevk annevk merged commit d5d0840 into whatwg:master Apr 27, 2018
@annevk
Copy link
Member

annevk commented Apr 27, 2018

Thanks @yoavweiss!

aarongable pushed a commit to chromium/chromium that referenced this pull request Apr 27, 2018
Currently `Request.destination` is set to "unknown" prefetch,
but that was recently changed:
Issue: whatwg/fetch#658
PR: whatwg/fetch#659

This CL aligns the destination values to the spec change.

Bug: 832105
Change-Id: Ib9f21dcc6cf0ace27b7a810d3670cddc45b3b74f
Reviewed-on: https://chromium-review.googlesource.com/1029858
Commit-Queue: Yoav Weiss <[email protected]>
Reviewed-by: Charlie Harrison <[email protected]>
Reviewed-by: Yutaka Hirano <[email protected]>
Cr-Commit-Position: refs/heads/master@{#554341}
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this pull request Apr 27, 2018
Currently `Request.destination` is set to "unknown" prefetch,
but that was recently changed:
Issue: whatwg/fetch#658
PR: whatwg/fetch#659

This CL aligns the destination values to the spec change.

Bug: 832105
Change-Id: Ib9f21dcc6cf0ace27b7a810d3670cddc45b3b74f
Reviewed-on: https://chromium-review.googlesource.com/1029858
Commit-Queue: Yoav Weiss <[email protected]>
Reviewed-by: Charlie Harrison <[email protected]>
Reviewed-by: Yutaka Hirano <[email protected]>
Cr-Commit-Position: refs/heads/master@{#554341}
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this pull request Apr 27, 2018
Currently `Request.destination` is set to "unknown" prefetch,
but that was recently changed:
Issue: whatwg/fetch#658
PR: whatwg/fetch#659

This CL aligns the destination values to the spec change.

Bug: 832105
Change-Id: Ib9f21dcc6cf0ace27b7a810d3670cddc45b3b74f
Reviewed-on: https://chromium-review.googlesource.com/1029858
Commit-Queue: Yoav Weiss <[email protected]>
Reviewed-by: Charlie Harrison <[email protected]>
Reviewed-by: Yutaka Hirano <[email protected]>
Cr-Commit-Position: refs/heads/master@{#554341}
moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this pull request May 2, 2018
…tonly

Automatic update from web-platform-testsAlign Request.destination to spec

Currently `Request.destination` is set to "unknown" prefetch,
but that was recently changed:
Issue: whatwg/fetch#658
PR: whatwg/fetch#659

This CL aligns the destination values to the spec change.

Bug: 832105
Change-Id: Ib9f21dcc6cf0ace27b7a810d3670cddc45b3b74f
Reviewed-on: https://chromium-review.googlesource.com/1029858
Commit-Queue: Yoav Weiss <[email protected]>
Reviewed-by: Charlie Harrison <[email protected]>
Reviewed-by: Yutaka Hirano <[email protected]>
Cr-Commit-Position: refs/heads/master@{#554341}

--

wpt-commits: 3c940291e9e8c30e8c7c401426160e3531639403
wpt-pr: 10657
gecko-dev-updater pushed a commit to marco-c/gecko-dev-comments-removed that referenced this pull request Oct 2, 2019
…tonly

Automatic update from web-platform-testsAlign Request.destination to spec

Currently `Request.destination` is set to "unknown" prefetch,
but that was recently changed:
Issue: whatwg/fetch#658
PR: whatwg/fetch#659

This CL aligns the destination values to the spec change.

Bug: 832105
Change-Id: Ib9f21dcc6cf0ace27b7a810d3670cddc45b3b74f
Reviewed-on: https://chromium-review.googlesource.com/1029858
Commit-Queue: Yoav Weiss <yoavyoav.ws>
Reviewed-by: Charlie Harrison <csharrisonchromium.org>
Reviewed-by: Yutaka Hirano <yhiranochromium.org>
Cr-Commit-Position: refs/heads/master{#554341}

--

wpt-commits: 3c940291e9e8c30e8c7c401426160e3531639403
wpt-pr: 10657

UltraBlame original commit: b60142c1cc9a01efbb27b591c1828015ffa0a6cb
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified that referenced this pull request Oct 3, 2019
…tonly

Automatic update from web-platform-testsAlign Request.destination to spec

Currently `Request.destination` is set to "unknown" prefetch,
but that was recently changed:
Issue: whatwg/fetch#658
PR: whatwg/fetch#659

This CL aligns the destination values to the spec change.

Bug: 832105
Change-Id: Ib9f21dcc6cf0ace27b7a810d3670cddc45b3b74f
Reviewed-on: https://chromium-review.googlesource.com/1029858
Commit-Queue: Yoav Weiss <yoavyoav.ws>
Reviewed-by: Charlie Harrison <csharrisonchromium.org>
Reviewed-by: Yutaka Hirano <yhiranochromium.org>
Cr-Commit-Position: refs/heads/master{#554341}

--

wpt-commits: 3c940291e9e8c30e8c7c401426160e3531639403
wpt-pr: 10657

UltraBlame original commit: b60142c1cc9a01efbb27b591c1828015ffa0a6cb
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified-and-comments-removed that referenced this pull request Oct 3, 2019
…tonly

Automatic update from web-platform-testsAlign Request.destination to spec

Currently `Request.destination` is set to "unknown" prefetch,
but that was recently changed:
Issue: whatwg/fetch#658
PR: whatwg/fetch#659

This CL aligns the destination values to the spec change.

Bug: 832105
Change-Id: Ib9f21dcc6cf0ace27b7a810d3670cddc45b3b74f
Reviewed-on: https://chromium-review.googlesource.com/1029858
Commit-Queue: Yoav Weiss <yoavyoav.ws>
Reviewed-by: Charlie Harrison <csharrisonchromium.org>
Reviewed-by: Yutaka Hirano <yhiranochromium.org>
Cr-Commit-Position: refs/heads/master{#554341}

--

wpt-commits: 3c940291e9e8c30e8c7c401426160e3531639403
wpt-pr: 10657

UltraBlame original commit: b60142c1cc9a01efbb27b591c1828015ffa0a6cb
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

4 participants