diff options
3 files changed, 39 insertions, 17 deletions
diff --git a/Libraries/LibJS/Runtime/Object.cpp b/Libraries/LibJS/Runtime/Object.cpp index f2fd036eaf..09649fc074 100644 --- a/Libraries/LibJS/Runtime/Object.cpp +++ b/Libraries/LibJS/Runtime/Object.cpp @@ -462,7 +462,10 @@ bool Object::put_own_property(Object& this_object, const FlyString& property_nam { ASSERT(!(mode == PutOwnPropertyMode::Put && value.is_accessor())); - if (!is_extensible()) { + auto metadata = shape().lookup(property_name); + bool new_property = !metadata.has_value(); + + if (!is_extensible() && new_property) { #ifdef OBJECT_DEBUG dbg() << "Disallow define_property of non-extensible object"; #endif @@ -479,8 +482,6 @@ bool Object::put_own_property(Object& this_object, const FlyString& property_nam attributes.set_has_setter(); } - auto metadata = shape().lookup(property_name); - bool new_property = !metadata.has_value(); if (new_property) { if (!m_shape->is_unique() && shape().property_count() > 100) { @@ -544,7 +545,10 @@ bool Object::put_own_property_by_index(Object& this_object, u32 property_index, { ASSERT(!(mode == PutOwnPropertyMode::Put && value.is_accessor())); - if (!is_extensible()) { + auto existing_property = m_indexed_properties.get(nullptr, property_index, false); + auto new_property = !existing_property.has_value(); + + if (!is_extensible() && new_property) { #ifdef OBJECT_DEBUG dbg() << "Disallow define_property of non-extensible object"; #endif @@ -561,8 +565,6 @@ bool Object::put_own_property_by_index(Object& this_object, u32 property_index, attributes.set_has_setter(); } - auto existing_property = m_indexed_properties.get(nullptr, property_index, false); - auto new_property = !existing_property.has_value(); PropertyAttributes existing_attributes = new_property ? 0 : existing_property.value().attributes; if (!new_property && mode == PutOwnPropertyMode::DefineProperty && !existing_attributes.is_configurable() && attributes != existing_attributes) { diff --git a/Libraries/LibJS/Tests/builtins/Object/Object.preventExtensions.js b/Libraries/LibJS/Tests/builtins/Object/Object.preventExtensions.js index 776fc81d3f..292c3f5999 100644 --- a/Libraries/LibJS/Tests/builtins/Object/Object.preventExtensions.js +++ b/Libraries/LibJS/Tests/builtins/Object/Object.preventExtensions.js @@ -24,6 +24,20 @@ describe("correct behavior", () => { o.baz = "baz"; expect(o.baz).toBeUndefined(); }); + + test("modifying existing properties", () => { + const o = { foo: "bar" }; + Object.preventExtensions(o); + o.foo = "baz"; + expect(o.foo).toBe("baz"); + }); + + test("deleting existing properties", () => { + const o = { foo: "bar" }; + Object.preventExtensions(o); + delete o.foo; + expect(o).not.toHaveProperty("foo"); + }); }); describe("errors", () => { diff --git a/Libraries/LibJS/Tests/builtins/Reflect/Reflect.preventExtensions.js b/Libraries/LibJS/Tests/builtins/Reflect/Reflect.preventExtensions.js index 0055ef7741..6b0bfdc1bc 100644 --- a/Libraries/LibJS/Tests/builtins/Reflect/Reflect.preventExtensions.js +++ b/Libraries/LibJS/Tests/builtins/Reflect/Reflect.preventExtensions.js @@ -1,7 +1,3 @@ -test("length is 1", () => { - expect(Reflect.preventExtensions).toHaveLength(1); -}); - describe("errors", () => { test("target must be an object", () => { [null, undefined, "foo", 123, NaN, Infinity].forEach(value => { @@ -16,6 +12,10 @@ describe("errors", () => { }); describe("normal behavior", () => { + test("length is 1", () => { + expect(Reflect.preventExtensions).toHaveLength(1); + }); + test("properties cannot be added", () => { var o = {}; o.foo = "foo"; @@ -25,12 +25,18 @@ describe("normal behavior", () => { expect(o.bar).toBeUndefined(); }); - test("property values can still be changed", () => { - // FIXME: This doesn't work even though it should (the value remains unchanged) - // var o = {}; - // o.foo = "foo"; - // expect(Reflect.preventExtensions(o)).toBeTrue(); - // o.foo = "bar"; - // expect(o.foo).toBe("bar"); + test("modifying existing properties", () => { + const o = {}; + o.foo = "foo"; + expect(Reflect.preventExtensions(o)).toBeTrue(); + o.foo = "bar"; + expect(o.foo).toBe("bar"); + }); + + test("deleting existing properties", () => { + const o = { foo: "bar" }; + Reflect.preventExtensions(o); + delete o.foo; + expect(o).not.toHaveProperty("foo"); }); }); |