From f991e40d7f49ce489b6115c777e1be7893addd46 Mon Sep 17 00:00:00 2001
From: Shannon Booth <shannon@serenityos.org>
Date: Sun, 3 Sep 2023 15:23:35 +1200
Subject: [PATCH] LibWeb: Support [Reflect] on IDL String attributes that may
 return null

This change allows IDL interfaces to be compiled using new AK String
which have a attribute in the interface that may return null.

Without this change we would run into a compile error from code such as
the following example:

```
auto retval = impl->deprecated_attribute(HTML::AttributeNames::ref);

if (!retval.has_value()) {
    return JS::js_null();
 }
 return JS::PrimitiveString::create(vm, retval.release_value());
```

As `deprecated_attribute` returns a `DeprecatedString` instead of an
`Optional<String>`. Fix that by using the non-deprecated attribute
implementation, and falling back to the empty string for where we cannot
return null.

Also add a test here to cover a regression I almost introduced here
which was not previously covered by our test suite.

Ideally, all of this should actually just be calling
Element::get_attribute_value, but I'm not entirely sure at this stage
what the behavioral change would be to test for here. Since this
implementation preserves the previous behavior, stick with it, and add a
FIXME for now.
---
 .../BindingsGenerator/IDLGenerators.cpp       | 30 +++++++++++++++++--
 .../idl-handling-of-null-attribute.txt        |  2 ++
 .../input/idl-handling-of-null-attribute.html |  9 ++++++
 3 files changed, 39 insertions(+), 2 deletions(-)
 create mode 100644 Tests/LibWeb/Text/expected/idl-handling-of-null-attribute.txt
 create mode 100644 Tests/LibWeb/Text/input/idl-handling-of-null-attribute.html

diff --git a/Meta/Lagom/Tools/CodeGenerators/LibWeb/BindingsGenerator/IDLGenerators.cpp b/Meta/Lagom/Tools/CodeGenerators/LibWeb/BindingsGenerator/IDLGenerators.cpp
index bbbdde381c..7d29ab0e8f 100644
--- a/Meta/Lagom/Tools/CodeGenerators/LibWeb/BindingsGenerator/IDLGenerators.cpp
+++ b/Meta/Lagom/Tools/CodeGenerators/LibWeb/BindingsGenerator/IDLGenerators.cpp
@@ -2450,9 +2450,22 @@ static void collect_attribute_values_of_an_inheritance_stack(SourceGenerator& fu
 
             if (attribute.extended_attributes.contains("Reflect")) {
                 if (attribute.type->name() != "boolean") {
-                    attribute_generator.append(R"~~~(
+                    if (interface_in_chain.extended_attributes.contains("UseDeprecatedAKString")) {
+                        attribute_generator.append(R"~~~(
     auto @attribute.return_value_name@ = impl->deprecated_attribute(HTML::AttributeNames::@attribute.reflect_name@);
 )~~~");
+                    } else {
+                        // FIXME: This should be calling Element::get_attribute_value: https://dom.spec.whatwg.org/#concept-element-attributes-get-value
+                        if (attribute.type->is_nullable()) {
+                            attribute_generator.append(R"~~~(
+    auto @attribute.return_value_name@ = impl->attribute(HTML::AttributeNames::@attribute.reflect_name@);
+)~~~");
+                        } else {
+                            attribute_generator.append(R"~~~(
+    auto @attribute.return_value_name@ = impl->attribute(HTML::AttributeNames::@attribute.reflect_name@).value_or(String {});
+)~~~");
+                        }
+                    }
                 } else {
                     attribute_generator.append(R"~~~(
     auto @attribute.return_value_name@ = impl->has_attribute(HTML::AttributeNames::@attribute.reflect_name@);
@@ -2776,9 +2789,22 @@ JS_DEFINE_NATIVE_FUNCTION(@class_name@::@attribute.getter_callback@)
 
         if (attribute.extended_attributes.contains("Reflect")) {
             if (attribute.type->name() != "boolean") {
-                attribute_generator.append(R"~~~(
+                if (interface.extended_attributes.contains("UseDeprecatedAKString")) {
+                    attribute_generator.append(R"~~~(
     auto retval = impl->deprecated_attribute(HTML::AttributeNames::@attribute.reflect_name@);
 )~~~");
+                } else {
+                    // FIXME: This should be calling Element::get_attribute_value: https://dom.spec.whatwg.org/#concept-element-attributes-get-value
+                    if (attribute.type->is_nullable()) {
+                        attribute_generator.append(R"~~~(
+    auto retval = impl->attribute(HTML::AttributeNames::@attribute.reflect_name@);
+)~~~");
+                    } else {
+                        attribute_generator.append(R"~~~(
+    auto retval = impl->attribute(HTML::AttributeNames::@attribute.reflect_name@).value_or(String {});
+)~~~");
+                    }
+                }
             } else {
                 attribute_generator.append(R"~~~(
     auto retval = impl->has_attribute(HTML::AttributeNames::@attribute.reflect_name@);
diff --git a/Tests/LibWeb/Text/expected/idl-handling-of-null-attribute.txt b/Tests/LibWeb/Text/expected/idl-handling-of-null-attribute.txt
new file mode 100644
index 0000000000..82700290a8
--- /dev/null
+++ b/Tests/LibWeb/Text/expected/idl-handling-of-null-attribute.txt
@@ -0,0 +1,2 @@
+initial type = ''
+after setting to null = 'null'
diff --git a/Tests/LibWeb/Text/input/idl-handling-of-null-attribute.html b/Tests/LibWeb/Text/input/idl-handling-of-null-attribute.html
new file mode 100644
index 0000000000..8a1d3d3eaa
--- /dev/null
+++ b/Tests/LibWeb/Text/input/idl-handling-of-null-attribute.html
@@ -0,0 +1,9 @@
+<script src="include.js"></script>
+<script id="script-element">
+    test(() => {
+        let s = document.getElementById("script-element");
+        println(`initial type = '${s.type}'`);
+        s.type = null;
+        println(`after setting to null = '${s.type}'`);
+    });
+</script>