Skip to content

Commit

Permalink
LibWeb: Implement indexed property support for HTML::Storage
Browse files Browse the repository at this point in the history
We only supported named properties on Storage, and as a result
`localStorage[0]` would be disconnected from the Storage's backing map.

Fixes at least 20 subtests in WPT in /webstorage.

(cherry picked from commit 4c189166f4f721211ff9b693f306f65033d70ac1)
  • Loading branch information
gmta authored and nico committed Nov 30, 2024
1 parent 091e832 commit d8c3dc0
Show file tree
Hide file tree
Showing 4 changed files with 42 additions and 0 deletions.
6 changes: 6 additions & 0 deletions Tests/LibWeb/Text/expected/localStorage.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
value
value
other
other
foo
foo
17 changes: 17 additions & 0 deletions Tests/LibWeb/Text/input/localStorage.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
<script src="include.js"></script>
<script>
test(() => {
localStorage["test"] = "value";
println(localStorage["test"]);
println(localStorage.getItem("test"));

localStorage.setItem("test", "other");
println(localStorage["test"]);
println(localStorage.getItem("test"));

// Ensure indexed properties work
localStorage[0] = "foo";
println(localStorage[0]);
println(localStorage.getItem("0"));
});
</script>
17 changes: 17 additions & 0 deletions Userland/Libraries/LibWeb/HTML/Storage.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -23,10 +23,13 @@ Storage::Storage(JS::Realm& realm)
: Bindings::PlatformObject(realm)
{
m_legacy_platform_object_flags = LegacyPlatformObjectFlags {
.supports_indexed_properties = true,
.supports_named_properties = true,
.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,
};
Expand Down Expand Up @@ -168,6 +171,13 @@ Vector<FlyString> Storage::supported_property_names() const
return names;
}

Optional<JS::Value> Storage::item_value(size_t index) const
{
// Handle index as a string since that's our key type
auto key = String::number(index);
return named_item_value(key);
}

JS::Value Storage::named_item_value(FlyString const& name) const
{
auto value = get_item(name);
Expand All @@ -182,6 +192,13 @@ WebIDL::ExceptionOr<Bindings::PlatformObject::DidDeletionFail> Storage::delete_v
return DidDeletionFail::NotRelevant;
}

WebIDL::ExceptionOr<void> Storage::set_value_of_indexed_property(u32 index, JS::Value unconverted_value)
{
// Handle index as a string since that's our key type
auto key = String::number(index);
return set_value_of_named_property(key, unconverted_value);
}

WebIDL::ExceptionOr<void> Storage::set_value_of_named_property(String const& key, JS::Value unconverted_value)
{
// NOTE: Since PlatformObject does not know the type of value, we must convert it ourselves.
Expand Down
2 changes: 2 additions & 0 deletions Userland/Libraries/LibWeb/HTML/Storage.h
Original file line number Diff line number Diff line change
Expand Up @@ -38,9 +38,11 @@ class Storage : public Bindings::PlatformObject {
virtual void initialize(JS::Realm&) override;

// ^PlatformObject
virtual Optional<JS::Value> item_value(size_t index) const override;
virtual JS::Value named_item_value(FlyString const&) const override;
virtual WebIDL::ExceptionOr<DidDeletionFail> delete_value(String const&) override;
virtual Vector<FlyString> supported_property_names() const override;
virtual WebIDL::ExceptionOr<void> set_value_of_indexed_property(u32, JS::Value) override;
virtual WebIDL::ExceptionOr<void> set_value_of_named_property(String const& key, JS::Value value) override;

void reorder();
Expand Down

0 comments on commit d8c3dc0

Please sign in to comment.