Bug 210326

Summary: module's default cross-origin value should be "anonymous"
Product: WebKit Reporter: Yusuke Suzuki <ysuzuki>
Component: New BugsAssignee: Yusuke Suzuki <ysuzuki>
Status: RESOLVED FIXED    
Severity: Normal CC: cdumez, commit-queue, domfarolino, esprehn+autocc, ews-watchlist, gyuyoung.kim, jaffathecake, japhet, kangil.han, ptoomey3, sam, webkit-bug-importer, webkitbugzilla, youennf
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
See Also: https://bugs.webkit.org/show_bug.cgi?id=210434
Bug Depends on: 210441    
Bug Blocks:    
Attachments:
Description Flags
Patch
none
Patch
none
Patch
none
Patch
none
Patch
none
Patch sam: review+

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. ***