Bug 210326 - module's default cross-origin value should be "anonymous"
Summary: module's default cross-origin value should be "anonymous"
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Yusuke Suzuki
URL:
Keywords: InRadar
: 171550 171566 189888 206811 (view as bug list)
Depends on: 210441
Blocks:
  Show dependency tree / graph
 
Reported: 2020-04-10 01:22 PDT by Yusuke Suzuki
Modified: 2024-03-17 08:07 PDT (History)
14 users (show)

See Also:

Attachments
Patch (3.56 KB, patch)
2020-04-10 01:24 PDT, Yusuke Suzuki 		no flags Details | Formatted Diff | Diff
Patch (20.89 KB, patch)
2020-04-12 10:30 PDT, Yusuke Suzuki 		no flags Details | Formatted Diff | Diff
Patch (38.38 KB, patch)
2020-04-12 11:02 PDT, Yusuke Suzuki 		no flags Details | Formatted Diff | Diff
Patch (52.68 KB, patch)
2020-04-12 16:55 PDT, Yusuke Suzuki 		no flags Details | Formatted Diff | Diff
Patch (61.86 KB, patch)
2020-04-12 16:59 PDT, Yusuke Suzuki 		no flags Details | Formatted Diff | Diff
Patch (31.04 KB, patch)
2020-04-13 12:33 PDT, Yusuke Suzuki 		sam: review+
Details | Formatted Diff | Diff
Show Obsolete (5) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Yusuke Suzuki 2020-04-10 01:22:29 PDT 
module's default cross-origin value should be "same-origin"
Comment 1 Yusuke Suzuki 2020-04-10 01:24:35 PDT 
Created attachment 396064 [details]
Patch
Comment 2 Yusuke Suzuki 2020-04-10 01:25:43 PDT 
<rdar://problem/51729982>
Comment 3 Yusuke Suzuki 2020-04-12 10:30:00 PDT 
Created attachment 396228 [details]
Patch
Comment 4 Yusuke Suzuki 2020-04-12 11:02:18 PDT 
Created attachment 396233 [details]
Patch
Comment 5 Sam Weinig 2020-04-12 11:52:01 PDT 
Comment on attachment 396233 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=396233&action=review

> Source/WebCore/ChangeLog:10
> +        The original spec was using "omit" crossorigin for modules when crossorigin is not set / empty.
> +        However, the spec is changed to sending requests with "same-origin" credentials mode by default.
> +        We should follow it.

Given the way "same-origin" is specified is as "anonymous", I think clarifying that in the change log would help make things clearer.

> Source/WebCore/ChangeLog:17
> +        * dom/ScriptElement.cpp:
> +        (WebCore::ScriptElement::requestModuleScript):
> +        * dom/ScriptElementCachedScriptFetcher.cpp:
> +        (WebCore::ScriptElementCachedScriptFetcher::requestModuleScript const):
> +        * html/parser/HTMLResourcePreloader.cpp:
> +        (WebCore::PreloadRequest::resourceRequest):

Its unfortunate this is in three places. Any ideas about how we could refactor to have a single place implementing this part of the spec?
Comment 6 Yusuke Suzuki 2020-04-12 16:42:35 PDT 
Comment on attachment 396233 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=396233&action=review

Thanks!

>> Source/WebCore/ChangeLog:10
>> +        We should follow it.
> 
> Given the way "same-origin" is specified is as "anonymous", I think clarifying that in the change log would help make things clearer.

Fixed.

>> Source/WebCore/ChangeLog:17
>> +        (WebCore::PreloadRequest::resourceRequest):
> 
> Its unfortunate this is in three places. Any ideas about how we could refactor to have a single place implementing this part of the spec?

Sounds nice, I'll put this string as  ScriptElementCachedScriptFetcher::defaultCrossOriginModeForModule to share.
Comment 7 Yusuke Suzuki 2020-04-12 16:55:25 PDT 
Created attachment 396245 [details]
Patch
Comment 8 Yusuke Suzuki 2020-04-12 16:59:29 PDT 
Created attachment 396246 [details]
Patch
Comment 9 Yusuke Suzuki 2020-04-13 06:10:53 PDT 
Committed r260003: <https://trac.webkit.org/changeset/260003>
Comment 10 Yusuke Suzuki 2020-04-13 06:12:20 PDT 
*** Bug 171550 has been marked as a duplicate of this bug. ***
Comment 11 WebKit Commit Bot 2020-04-13 10:53:40 PDT 
Re-opened since this is blocked by bug 210441
Comment 12 Yusuke Suzuki 2020-04-13 12:33:10 PDT 
Created attachment 396316 [details]
Patch
Comment 13 Yusuke Suzuki 2020-04-13 12:38:24 PDT 
Comment on attachment 396316 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=396316&action=review

> Source/WebCore/ChangeLog:12
> +

C++ part is not changed. Just rewrite tests with cookie instead of basic-authentication since basic-authentication-based tests hit some existing crashes in WK2-Debug bots.
Comment 14 Yusuke Suzuki 2020-04-13 13:24:46 PDT 
Thanks Sam! I'll land it once I checked that EWS is green.
Comment 15 Yusuke Suzuki 2020-04-13 13:54:06 PDT 
Bots are green. Landing.
Comment 16 Yusuke Suzuki 2020-04-13 13:59:15 PDT 
Committed r260038: <https://trac.webkit.org/changeset/260038>
Comment 17 Yusuke Suzuki 2020-04-14 10:03:48 PDT 
*** Bug 206811 has been marked as a duplicate of this bug. ***
Comment 18 Yusuke Suzuki 2020-05-20 11:38:42 PDT 
*** Bug 171566 has been marked as a duplicate of this bug. ***
Comment 19 Anne van Kesteren 2024-03-17 08:07:24 PDT 
*** Bug 189888 has been marked as a duplicate of this bug. ***