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

Module scripts use same-origin credentials mode by default #13176

Merged
merged 1 commit into from Sep 26, 2018

Conversation

Projects
None yet
4 participants
@chromium-wpt-export-bot
Collaborator

chromium-wpt-export-bot commented Sep 23, 2018

Before this CL, module scripts via <script type=module> used "omit" as
the default credentials mode. After this CL, "same-origin" is used. This
extends to module script descendants as well. Intent to implement and
ship: https://groups.google.com/a/chromium.org/forum/#!topic/blink-dev/CUAxbvtnCh4.

R=kinuko@chromium.org, kouhei@chromium.org, nhiroki@chromium.org

Bug: 849101
Change-Id: I62617aafd3e226bc86459ec4a24138d9eac6e8ff
Reviewed-on: https://chromium-review.googlesource.com/1239638
Reviewed-by: Kouhei Ueno <kouhei@chromium.org>
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Reviewed-by: Hiroki Nakagawa <nhiroki@chromium.org>
Commit-Queue: Dominic Farolino <domfarolino@gmail.com>
Cr-Commit-Position: refs/heads/master@{#594305}

@wpt-pr-bot

Already reviewed downstream.

Module scripts use same-origin credentials mode by default
Before this CL, module scripts via <script type=module> used "omit" as
the default credentials mode. After this CL, "same-origin" is used. This
extends to module script descendants as well. Intent to implement and
ship: https://groups.google.com/a/chromium.org/forum/#!topic/blink-dev/CUAxbvtnCh4.

R=kinuko@chromium.org, kouhei@chromium.org, nhiroki@chromium.org

Bug: 849101
Change-Id: I62617aafd3e226bc86459ec4a24138d9eac6e8ff
Reviewed-on: https://chromium-review.googlesource.com/1239638
Reviewed-by: Kouhei Ueno <kouhei@chromium.org>
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Reviewed-by: Hiroki Nakagawa <nhiroki@chromium.org>
Commit-Queue: Dominic Farolino <domfarolino@gmail.com>
Cr-Commit-Position: refs/heads/master@{#594305}

@chromium-wpt-export-bot chromium-wpt-export-bot merged commit 9d582d8 into master Sep 26, 2018

2 checks passed

Taskcluster (pull_request) TaskGroup: success
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@chromium-wpt-export-bot chromium-wpt-export-bot deleted the chromium-export-cl-1239638 branch Sep 26, 2018

@bzbarsky

This comment has been minimized.

Show comment
Hide comment
@bzbarsky

bzbarsky Sep 27, 2018

Contributor

@domfarolino As far as I can tell, this test will now always fail in a UA that doesn't support the not-yet-final dynamic import proposal. Why was that change made? If we do want to test the dynamic import bits, shouldn't they be in a separate test, or at least a separate promise_test()?

Contributor

bzbarsky commented Sep 27, 2018

@domfarolino As far as I can tell, this test will now always fail in a UA that doesn't support the not-yet-final dynamic import proposal. Why was that change made? If we do want to test the dynamic import bits, shouldn't they be in a separate test, or at least a separate promise_test()?

@bzbarsky

This comment has been minimized.

Show comment
Hide comment
@bzbarsky

bzbarsky Sep 27, 2018

Contributor

(And in particular, the test became useless for the purpose of testing whether my fix for https://bugzilla.mozilla.org/show_bug.cgi?id=1493449 is actually correct...)

Contributor

bzbarsky commented Sep 27, 2018

(And in particular, the test became useless for the purpose of testing whether my fix for https://bugzilla.mozilla.org/show_bug.cgi?id=1493449 is actually correct...)

@domfarolino

This comment has been minimized.

Show comment
Hide comment
@domfarolino

domfarolino Sep 27, 2018

Member

Oh, I'm really sorry - that was a sloppy oversight. I'll move the dynamic import tests out to a separate file alongside the original. Apologies.

Member

domfarolino commented Sep 27, 2018

Oh, I'm really sorry - that was a sloppy oversight. I'll move the dynamic import tests out to a separate file alongside the original. Apologies.

@bzbarsky

This comment has been minimized.

Show comment
Hide comment
@bzbarsky

bzbarsky Sep 27, 2018

Contributor

No need to apologize. I really appreciate you splitting those tests out!

Contributor

bzbarsky commented Sep 27, 2018

No need to apologize. I really appreciate you splitting those tests out!

moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this pull request Oct 3, 2018

Bug 1493449. Change the default credentials mode for module scripts f…
…rom 'omit' to 'same-origin'. r=farre

The tests come directly from
web-platform-tests/wpt#13176 and
web-platform-tests/wpt#13245

Differential Revision: https://phabricator.services.mozilla.com/D7113

--HG--
extra : moz-landing-system : lando

mykmelez pushed a commit to mozilla/gecko that referenced this pull request Oct 3, 2018

moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this pull request Oct 6, 2018

Bug 1493449. Change the default credentials mode for module scripts f…
…rom 'omit' to 'same-origin'. r=farre

The tests come directly from
web-platform-tests/wpt#13176 and
web-platform-tests/wpt#13245

Differential Revision: https://phabricator.services.mozilla.com/D7113

--HG--
extra : moz-landing-system : lando

xeonchen pushed a commit to xeonchen/gecko that referenced this pull request Oct 8, 2018

domenic added a commit to whatwg/html that referenced this pull request Oct 9, 2018

Change how module scripts are fetched
* Module scripts are always fetched with request credentials mode
  "same-origin" by default, instead of the previous default of "omit".
  Only worker module scripts can still set that to "omit", using the
  credentials option to the Worker constructor. Non-worker module
  scripts, which only have the crossorigin="" attribute available, can
  only toggle between "same-origin" and "include", similar to how
  crossorigin="" works for other platform features.
* Similarly, import() statements inside of classic scripts now use the
  "same-origin" credentials mode, instead of "omit". This affects both
  <script> elements, where the default can be changed using
  crossorigin="", and other contexts like javascript: URLs and classic
  worker scripts, where the default cannot be changed.
* The top-level script for module workers is always fetched with request
  mode "same-origin". Cross-origin workers did not quite work due to
  service workers.

Fixes #2557. Fixes #3109.

Tests:

* web-platform-tests/wpt#11274
* web-platform-tests/wpt#13176
* web-platform-tests/wpt#13426
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment