Skip to content

Commit

Permalink
Align with HTML and remove support for external importmaps
Browse files Browse the repository at this point in the history
https://bugs.webkit.org/show_bug.cgi?id=284286

Reviewed by Yusuke Suzuki.

This PR removes support for external import maps and ensures they throw an
error instead. That would align WebKit's behavior with the HTML standard
and other browsers.

It also imports a new WPT test that validates the standard requirement
this fixes, as well as adjusts relevant WPT results. This adjustment
results in some progressions and some regressions. The regressions would
be fixed as part of
WebKit/standards-positions#381.

Upstream commit: web-platform-tests/wpt@ef41368

* LayoutTests/imported/w3c/web-platform-tests/import-maps/external-import-map-errors-expected.txt: Added.
* LayoutTests/imported/w3c/web-platform-tests/import-maps/external-import-map-errors.html: Added.
* LayoutTests/imported/w3c/web-platform-tests/import-maps/w3c-import.log:
* Source/WebCore/Sources.txt: remove LoadableImportMap
* Source/WebCore/WebCore.xcodeproj/project.pbxproj: remove LoadableImportMap
* Source/WebCore/dom/LoadableClassicScript.cpp: remove LoadableImportMap
  logic
* Source/WebCore/dom/LoadableImportMap.cpp: Removed.
* Source/WebCore/dom/LoadableImportMap.h: Removed.
(WebCore::ScriptElement::prepareScript): Fire an error event if
an importmap script has a source.
(WebCore::ScriptElement::executePendingScript): remove
  loadableImportMap logic
(WebCore::ScriptElement::requestImportMap): Deleted.
* Source/WebCore/dom/ScriptElement.h: Remove `requestImportMap`.

Canonical link: https://commits.webkit.org/287725@main
  • Loading branch information
yoavweiss committed Dec 12, 2024
1 parent 6390ac1 commit 6c9bf5a
Show file tree
Hide file tree
Showing 10 changed files with 51 additions and 165 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@

PASS Test that an external import map fires an error event
PASS Test that an external import map fires an error event, regardless of attribute order
PASS Test that an external import map in markup fires an error event

Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
<!DOCTYPE html>
<head>
<script src="/resources/testharness.js"></script>
<script src="/resources/testharnessreport.js"></script>
<script>
window.markupErrored = false;
window.markupLoaded = false;
</script>
<script src="/import-maps/resources/empty.js" type="importmap"
onload="window.markupLoaded = true;" onerror="window.markupErrored = true;">
</script>
<script>
promise_test(async () => {
await new Promise((resolve, reject) => {
const element = document.createElement("script");
element.onload = () => { reject("Got an unexpected load event"); };
element.onerror = () => { resolve("Got an error event"); };
element.src = "/import-maps/resources/empty.js";
element.type = "importmap";
document.head.appendChild(element);
})
}, "Test that an external import map fires an error event");

promise_test(async () => {
await new Promise((resolve, reject) => {
const element = document.createElement("script");
element.type = "importmap";
element.onload = () => { reject("Got an unexpected load event"); };
element.onerror = () => { resolve("Got an error event"); };
element.src = "/import-maps/resources/empty.js";
document.head.appendChild(element);
})
}, "Test that an external import map fires an error event, regardless of attribute order");

promise_test(async () => {
assert_true(window.markupErrored, "error");
assert_false(window.markupLoaded, "load");
}, "Test that an external import map in markup fires an error event");
</script>
</head>
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ List of files:
/LayoutTests/imported/w3c/web-platform-tests/import-maps/bare-specifiers.sub.html
/LayoutTests/imported/w3c/web-platform-tests/import-maps/data-url-specifiers.sub.html
/LayoutTests/imported/w3c/web-platform-tests/import-maps/dynamic-integrity.html
/LayoutTests/imported/w3c/web-platform-tests/import-maps/external-import-map-errors.html
/LayoutTests/imported/w3c/web-platform-tests/import-maps/http-url-like-specifiers.sub.html
/LayoutTests/imported/w3c/web-platform-tests/import-maps/import-maps-base-url.sub.html
/LayoutTests/imported/w3c/web-platform-tests/import-maps/module-map-key.html
Expand Down
1 change: 0 additions & 1 deletion Source/WebCore/Sources.txt
Original file line number Diff line number Diff line change
Expand Up @@ -1294,7 +1294,6 @@ dom/InternalObserverTake.cpp
dom/KeyboardEvent.cpp
dom/LiveNodeList.cpp
dom/LoadableClassicScript.cpp
dom/LoadableImportMap.cpp
dom/LoadableModuleScript.cpp
dom/LoadableScript.cpp
dom/MessageChannel.cpp
Expand Down
6 changes: 0 additions & 6 deletions Source/WebCore/WebCore.xcodeproj/project.pbxproj
Original file line number Diff line number Diff line change
Expand Up @@ -6072,7 +6072,6 @@
E3565B7B1DC2D6C900217DBD /* JSEventCustom.h in Headers */ = {isa = PBXBuildFile; fileRef = E34EE49F1DC2D57500EAA9D3 /* JSEventCustom.h */; settings = {ATTRIBUTES = (Private, ); }; };
E35802B61DC8435D00A9773C /* DOMJITIDLTypeFilter.h in Headers */ = {isa = PBXBuildFile; fileRef = E35802B51DC8435800A9773C /* DOMJITIDLTypeFilter.h */; settings = {ATTRIBUTES = (Private, ); }; };
E3582C282527F66900D1B790 /* WebCoreJITOperations.h in Headers */ = {isa = PBXBuildFile; fileRef = E3582C262527F66800D1B790 /* WebCoreJITOperations.h */; settings = {ATTRIBUTES = (Private, ); }; };
E35B8C6228DD204100293D90 /* LoadableImportMap.h in Headers */ = {isa = PBXBuildFile; fileRef = E35B8C5E28DD202D00293D90 /* LoadableImportMap.h */; settings = {ATTRIBUTES = (Private, ); }; };
E35B907F23F60A50000011FF /* LocalizedDeviceModel.h in Headers */ = {isa = PBXBuildFile; fileRef = E35B907C23F60677000011FF /* LocalizedDeviceModel.h */; settings = {ATTRIBUTES = (Private, ); }; };
E364321C25D37A6700F90E2A /* ModuleScriptLoader.h in Headers */ = {isa = PBXBuildFile; fileRef = E364321A25D37A6600F90E2A /* ModuleScriptLoader.h */; settings = {ATTRIBUTES = (Private, ); }; };
E36A00E429CF7B8600AC4E8A /* TextTrackRepresentationCocoa.h in Headers */ = {isa = PBXBuildFile; fileRef = 526724F21CB2FDF60075974D /* TextTrackRepresentationCocoa.h */; settings = {ATTRIBUTES = (Private, ); }; };
Expand Down Expand Up @@ -20316,8 +20315,6 @@
E35802B51DC8435800A9773C /* DOMJITIDLTypeFilter.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = DOMJITIDLTypeFilter.h; sourceTree = "<group>"; };
E3582C242527F66800D1B790 /* WebCoreJITOperations.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = WebCoreJITOperations.cpp; sourceTree = "<group>"; };
E3582C262527F66800D1B790 /* WebCoreJITOperations.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = WebCoreJITOperations.h; sourceTree = "<group>"; };
E35B8C5E28DD202D00293D90 /* LoadableImportMap.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = LoadableImportMap.h; sourceTree = "<group>"; };
E35B8C5F28DD202D00293D90 /* LoadableImportMap.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = LoadableImportMap.cpp; sourceTree = "<group>"; };
E35B907C23F60677000011FF /* LocalizedDeviceModel.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = LocalizedDeviceModel.h; sourceTree = "<group>"; };
E35B907E23F60677000011FF /* LocalizedDeviceModel.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; path = LocalizedDeviceModel.mm; sourceTree = "<group>"; };
E364321A25D37A6600F90E2A /* ModuleScriptLoader.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = ModuleScriptLoader.h; sourceTree = "<group>"; };
Expand Down Expand Up @@ -38382,8 +38379,6 @@
CD814C3D2996FAE8005A780A /* LiveNodeListInlines.h */,
E3B2F0E31D7F35EC00B0C9D1 /* LoadableClassicScript.cpp */,
E3B2F0E41D7F35EC00B0C9D1 /* LoadableClassicScript.h */,
E35B8C5F28DD202D00293D90 /* LoadableImportMap.cpp */,
E35B8C5E28DD202D00293D90 /* LoadableImportMap.h */,
E307DED21D81E4ED00141CAF /* LoadableModuleScript.cpp */,
E307DED31D81E4ED00141CAF /* LoadableModuleScript.h */,
E3B2F0E91D7F3D3C00B0C9D1 /* LoadableScript.cpp */,
Expand Down Expand Up @@ -42420,7 +42415,6 @@
BC7FA6210D1F0CBD00DB22A9 /* LiveNodeList.h in Headers */,
CD814C3E2996FAE8005A780A /* LiveNodeListInlines.h in Headers */,
E3B2F0F01D7F4CB500B0C9D1 /* LoadableClassicScript.h in Headers */,
E35B8C6228DD204100293D90 /* LoadableImportMap.h in Headers */,
E307DED51D81E4F200141CAF /* LoadableModuleScript.h in Headers */,
E3B2F0ED1D7F4CA300B0C9D1 /* LoadableScript.h in Headers */,
E3B2F0EE1D7F4CA900B0C9D1 /* LoadableScriptClient.h in Headers */,
Expand Down
1 change: 0 additions & 1 deletion Source/WebCore/dom/LoadableClassicScript.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@
#include "DefaultResourceLoadPriority.h"
#include "Element.h"
#include "FetchIdioms.h"
#include "LoadableImportMap.h"
#include "LoadableScriptError.h"
#include "ScriptElement.h"
#include "ScriptSourceCode.h"
Expand Down
56 changes: 0 additions & 56 deletions Source/WebCore/dom/LoadableImportMap.cpp

This file was deleted.

57 changes: 0 additions & 57 deletions Source/WebCore/dom/LoadableImportMap.h

This file was deleted.

48 changes: 5 additions & 43 deletions Source/WebCore/dom/ScriptElement.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,6 @@
#include "IgnoreDestructiveWriteCountIncrementer.h"
#include "InlineClassicScript.h"
#include "LoadableClassicScript.h"
#include "LoadableImportMap.h"
#include "LoadableModuleScript.h"
#include "LocalFrame.h"
#include "MIMETypeRegistry.h"
Expand Down Expand Up @@ -266,11 +265,11 @@ bool ScriptElement::prepareScript(const TextPosition& scriptStartPosition)
}
frame->script().setAcquiringImportMaps();
if (hasSourceAttribute()) {
if (!requestImportMap(*frame, sourceAttributeValue()))
return false;
potentiallyBlockRendering();
} else
frame->script().setPendingImportMaps();
element->document().eventLoop().queueTask(TaskSource::DOMManipulation, [this, element] {
dispatchErrorEvent();
});
return false;
}
break;
}
}
Expand Down Expand Up @@ -419,39 +418,6 @@ bool ScriptElement::requestModuleScript(const TextPosition& scriptStartPosition)
return true;
}

bool ScriptElement::requestImportMap(LocalFrame& frame, const String& sourceURL)
{
Ref element = this->element();
Ref document = element->document();

ASSERT(element->isConnected());
ASSERT(!m_loadableScript);
if (!StringView(sourceURL).containsOnly<isASCIIWhitespace<UChar>>()) {
Ref script = LoadableImportMap::create(element->nonce(), element->attributeWithoutSynchronization(HTMLNames::integrityAttr), referrerPolicy(),
element->attributeWithoutSynchronization(HTMLNames::crossoriginAttr), element->localName(), element->isInUserAgentShadowTree(), hasAsyncAttribute());

auto scriptURL = document->completeURL(sourceURL);
document->willLoadScriptElement(scriptURL);

if (!document->checkedContentSecurityPolicy()->allowNonParserInsertedScripts(scriptURL, URL(), m_startLineNumber, element->nonce(), script->integrity(), String(), m_parserInserted))
return false;

frame.checkedScript()->setPendingImportMaps();
if (script->load(document, scriptURL)) {
m_loadableScript = WTFMove(script);
m_isExternalScript = true;
}
}

if (m_loadableScript)
return true;

document->checkedEventLoop()->queueTask(TaskSource::DOMManipulation, [this, element] {
dispatchErrorEvent();
});
return false;
}

void ScriptElement::executeClassicScript(const ScriptSourceCode& sourceCode)
{
RELEASE_ASSERT_WITH_SECURITY_IMPLICATION(ScriptDisallowedScope::InMainThread::isScriptAllowed());
Expand Down Expand Up @@ -609,10 +575,6 @@ void ScriptElement::executePendingScript(PendingScript& pendingScript)
RefPtr<Document> document { &element().document() };
if (document->identifier() != m_preparationTimeDocumentIdentifier) {
document->addConsoleMessage(MessageSource::Security, MessageLevel::Error, "Not executing script because it moved between documents during fetching"_s);
if (loadableScript) {
if (auto* loadableImportMap = dynamicDowncast<LoadableImportMap>(loadableScript))
document = loadableImportMap->document();
}
} else {
if (loadableScript)
executeScriptAndDispatchEvent(*loadableScript);
Expand Down
1 change: 0 additions & 1 deletion Source/WebCore/dom/ScriptElement.h
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,6 @@ class ScriptElement {

bool requestClassicScript(const String& sourceURL);
bool requestModuleScript(const TextPosition& scriptStartPosition);
bool requestImportMap(LocalFrame&, const String& sourceURL);

void updateTaintedOriginFromSourceURL();

Expand Down

0 comments on commit 6c9bf5a

Please sign in to comment.