From faf6fd11894ac1c1d8aeeb35cb3723c66f900f1a Mon Sep 17 00:00:00 2001
From: Luke Wilde <lukew@serenityos.org>
Date: Tue, 12 Nov 2024 12:13:41 +0000
Subject: [PATCH] LibWeb: Remove LegacyOverrideBuiltIns flag from Storage

This was preventing https://ubereats.com/ from fully loading, because
they are attempting to overwrite setItem. They seem to be trying to add
error logging to setItem if it throws, as all they do is add a
try/catch block that emits an error log to their monitoring service if
it throws.

However, because Storage is a legacy platform object with a named
property setter (setItem), it will call setItem with the stringified
version of the function. This is actually expected as per the spec,
Firefox (Gecko) and Epiphany (WebKit) does this too, but Chromium does
not as it actually overwrites the function with the new function and
does not store the stringified function.

The problem is that we had the LegacyOverrideBuiltIns flag accidentally
set, so it would return the stored string instead of the built-in
function (hence the name), then it would try and call it and throw a
"not a function" error. This prevented their JS from going any further.

This fix allows their UI to fully load and be fully interactive, though
it is quite slow at the moment!
---
 Libraries/LibWeb/HTML/Storage.cpp                  |  1 -
 ...does-not-have-legacy-override-builtins-flag.txt | 10 ++++++++++
 ...oes-not-have-legacy-override-builtins-flag.html | 14 ++++++++++++++
 3 files changed, 24 insertions(+), 1 deletion(-)
 create mode 100644 Tests/LibWeb/Text/expected/HTML/storage-does-not-have-legacy-override-builtins-flag.txt
 create mode 100644 Tests/LibWeb/Text/input/HTML/storage-does-not-have-legacy-override-builtins-flag.html

diff --git a/Libraries/LibWeb/HTML/Storage.cpp b/Libraries/LibWeb/HTML/Storage.cpp
index 6cf537b2e7f..c5d1a04f967 100644
--- a/Libraries/LibWeb/HTML/Storage.cpp
+++ b/Libraries/LibWeb/HTML/Storage.cpp
@@ -28,7 +28,6 @@ Storage::Storage(JS::Realm& realm)
         .has_indexed_property_setter = true,
         .has_named_property_setter = true,
         .has_named_property_deleter = true,
-        .has_legacy_override_built_ins_interface_extended_attribute = true,
         .indexed_property_setter_has_identifier = true,
         .named_property_setter_has_identifier = true,
         .named_property_deleter_has_identifier = true,
diff --git a/Tests/LibWeb/Text/expected/HTML/storage-does-not-have-legacy-override-builtins-flag.txt b/Tests/LibWeb/Text/expected/HTML/storage-does-not-have-legacy-override-builtins-flag.txt
new file mode 100644
index 00000000000..0a45f632ceb
--- /dev/null
+++ b/Tests/LibWeb/Text/expected/HTML/storage-does-not-have-legacy-override-builtins-flag.txt
@@ -0,0 +1,10 @@
+key should still be native: 'function key() { [native code] }'
+key's stringified function was added to storage: 'function () { println(`FAIL: Overriden ${functionName} was called`); }'
+getItem should still be native: 'function getItem() { [native code] }'
+getItem's stringified function was added to storage: 'function () { println(`FAIL: Overriden ${functionName} was called`); }'
+setItem should still be native: 'function setItem() { [native code] }'
+setItem's stringified function was added to storage: 'function () { println(`FAIL: Overriden ${functionName} was called`); }'
+removeItem should still be native: 'function removeItem() { [native code] }'
+removeItem's stringified function was added to storage: 'function () { println(`FAIL: Overriden ${functionName} was called`); }'
+clear should still be native: 'function clear() { [native code] }'
+clear's stringified function was added to storage: 'function () { println(`FAIL: Overriden ${functionName} was called`); }'
diff --git a/Tests/LibWeb/Text/input/HTML/storage-does-not-have-legacy-override-builtins-flag.html b/Tests/LibWeb/Text/input/HTML/storage-does-not-have-legacy-override-builtins-flag.html
new file mode 100644
index 00000000000..c1435cff819
--- /dev/null
+++ b/Tests/LibWeb/Text/input/HTML/storage-does-not-have-legacy-override-builtins-flag.html
@@ -0,0 +1,14 @@
+<!DOCTYPE html>
+<script src="../include.js"></script>
+<script>
+    test(() => {
+        for (const functionName of ["key", "getItem", "setItem", "removeItem", "clear"]) {
+            const failFunction = function () { println(`FAIL: Overriden ${functionName} was called`); };
+            window.localStorage[functionName] = failFunction;
+
+            println(`${functionName} should still be native: '${window.localStorage[functionName].toString()}'`);
+            println(`${functionName}'s stringified function was added to storage: '${window.localStorage.getItem(functionName)}'`);
+            window.localStorage[functionName]("1", "2");
+        }
+    });
+</script>