From 5611285312e1125a0daf3d6e4c116bb0180527bd Mon Sep 17 00:00:00 2001
From: davidot <david.tuin@gmail.com>
Date: Thu, 30 Sep 2021 00:36:28 +0200
Subject: SpreadSheet: Fix that the js integration abused global objects

Before this commit it only allocated the global object so when it wanted
to lookup 'thisSheet' it could not find it in the global environment.
We now hotswap the global object everytime a cell evaluated.

This also fixes that SheetGlobalObject did not have an
internal_has_property meaning 'A0' could not be referenced unless it was
via a member lookup (this.A0). This was already broken before the
bindings refactoring.

The correct behavior of realms in spreadsheet is not completely clear
since what is shared between sheets is not very well defined.

The reason that just setting the SheetGlobalObject as the
global_this_value is not enough is because ECMAScript does not check the
global_this_value for members when resolving a reference in the global
environment.
---
 Userland/Applications/Spreadsheet/JSIntegration.cpp | 12 +++++++++++-
 Userland/Applications/Spreadsheet/JSIntegration.h   |  1 +
 Userland/Applications/Spreadsheet/Spreadsheet.cpp   |  3 +++
 3 files changed, 15 insertions(+), 1 deletion(-)

(limited to 'Userland/Applications/Spreadsheet')

diff --git a/Userland/Applications/Spreadsheet/JSIntegration.cpp b/Userland/Applications/Spreadsheet/JSIntegration.cpp
index 54a822872d..565a8ad942 100644
--- a/Userland/Applications/Spreadsheet/JSIntegration.cpp
+++ b/Userland/Applications/Spreadsheet/JSIntegration.cpp
@@ -8,7 +8,6 @@
 #include "Spreadsheet.h"
 #include "Workbook.h"
 #include <LibJS/Lexer.h>
-#include <LibJS/Runtime/Completion.h>
 #include <LibJS/Runtime/Error.h>
 #include <LibJS/Runtime/GlobalObject.h>
 #include <LibJS/Runtime/Object.h>
@@ -102,6 +101,17 @@ SheetGlobalObject::~SheetGlobalObject()
 {
 }
 
+JS::ThrowCompletionOr<bool> SheetGlobalObject::internal_has_property(JS::PropertyName const& name) const
+{
+    if (name.is_string()) {
+        if (name.as_string() == "value")
+            return true;
+        if (m_sheet.parse_cell_name(name.as_string()).has_value())
+            return true;
+    }
+    return Object::internal_has_property(name);
+}
+
 JS::ThrowCompletionOr<JS::Value> SheetGlobalObject::internal_get(const JS::PropertyName& property_name, JS::Value receiver) const
 {
     if (property_name.is_string()) {
diff --git a/Userland/Applications/Spreadsheet/JSIntegration.h b/Userland/Applications/Spreadsheet/JSIntegration.h
index 2f7d81b160..8bd1e09a40 100644
--- a/Userland/Applications/Spreadsheet/JSIntegration.h
+++ b/Userland/Applications/Spreadsheet/JSIntegration.h
@@ -27,6 +27,7 @@ public:
 
     virtual ~SheetGlobalObject() override;
 
+    virtual JS::ThrowCompletionOr<bool> internal_has_property(JS::PropertyName const& name) const override;
     virtual JS::ThrowCompletionOr<JS::Value> internal_get(JS::PropertyName const&, JS::Value receiver) const override;
     virtual JS::ThrowCompletionOr<bool> internal_set(JS::PropertyName const&, JS::Value value, JS::Value receiver) override;
     virtual void initialize_global_object() override;
diff --git a/Userland/Applications/Spreadsheet/Spreadsheet.cpp b/Userland/Applications/Spreadsheet/Spreadsheet.cpp
index 2d8aa751c5..079104ddf3 100644
--- a/Userland/Applications/Spreadsheet/Spreadsheet.cpp
+++ b/Userland/Applications/Spreadsheet/Spreadsheet.cpp
@@ -163,6 +163,9 @@ Sheet::ValueAndException Sheet::evaluate(const StringView& source, Cell* on_beha
     if (parser.has_errors() || interpreter().exception())
         return { JS::js_undefined(), interpreter().exception() };
 
+    // FIXME: This creates a GlobalEnvironment for every evaluate call which we might be able to circumvent with multiple realms.
+    interpreter().realm().set_global_object(global_object(), &global_object());
+
     interpreter().run(global_object(), program);
     if (interpreter().exception()) {
         auto exc = interpreter().exception();
-- 
cgit v1.2.3