Skip to content

Commit

Permalink
Fix #7169: rewrite assert_readonly to check property descriptors
Browse files Browse the repository at this point in the history
This changes the implementation to only check that the property
descriptor makes the property readonly; it doesn't actually check its
behaviour. As detailed in the above issue, there's too many ways for
this test to be otherwise bogus, thus we should go for the simple
implementation.

This also fixes /css/cssom/cssimportrule.html, which previously
asserted that CSSImportRule#media was readonly, which was a
meaningless test, as even after the [PutForwards] [[Set]] call had
occured, the attribute being tested still returned the same object,
because it is defined with [SameObject].
  • Loading branch information
gsnedders committed Nov 6, 2024
1 parent 3ad72e6 commit 877d18d
Show file tree
Hide file tree
Showing 2 changed files with 34 additions and 19 deletions.
1 change: 0 additions & 1 deletion css/cssom/cssimportrule.html
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,6 @@
assert_idl_attribute(rule, "styleSheet");

assert_readonly(rule, "href");
assert_readonly(rule, "media");
assert_readonly(rule, "styleSheet");
}, "Existence and writability of CSSImportRule attributes");

Expand Down
52 changes: 34 additions & 18 deletions resources/testharness.js
Original file line number Diff line number Diff line change
Expand Up @@ -2032,30 +2032,46 @@


/**
* Assert that ``object`` has a property named ``property_name`` and that the property is readonly.
* Assert that ``object`` has a property named ``property_name`` and that the property is not writable or has no setter.
*
* Note: The implementation tries to update the named property, so
* any side effects of updating will be triggered. Users are
* encouraged to instead inspect the property descriptor of ``property_name`` on ``object``.
*
* @param {Object} object - Object that should have the given property in its prototype chain.
* @param {Object} object - Object that should have the given (not necessarily own) property.
* @param {string} property_name - Expected property name.
* @param {string} [description] - Description of the condition being tested.
*/
function assert_readonly(object, property_name, description)
{
var initial_value = object[property_name];
try {
//Note that this can have side effects in the case where
//the property has PutForwards
object[property_name] = initial_value + "a"; //XXX use some other value here?
assert(same_value(object[property_name], initial_value),
"assert_readonly", description,
"changing property ${p} succeeded",
{p:property_name});
} finally {
object[property_name] = initial_value;
}
assert(property_name in object,
"assert_readonly", description,
"property ${p} not found",
{p:property_name});

let desc;
while (object && (desc = Object.getOwnPropertyDescriptor(object, property_name)) === undefined) {
object = Object.getPrototypeOf(object);
}

assert(desc !== undefined,
"assert_readonly", description,
"could not find a descriptor for property ${p}",
{p:property_name});

if (desc.hasOwnProperty("value")) {
// We're a data property descriptor
assert(desc.writable === false, "assert_readonly", description,
"descriptor [[Writable]] expected false got ${actual}", {actual:desc.writable});
} else if (desc.hasOwnProperty("get") || desc.hasOwnProperty("set")) {
// We're an accessor property descriptor
assert(desc.set === undefined, "assert_readonly", description,
"property ${p} is an accessor property with a [[Set]] attribute, cannot test readonly-ness",
{p:property_name});
} else {
// We're a generic property descriptor
// This shouldn't happen, because Object.getOwnPropertyDescriptor
// forwards the return value of [[GetOwnProperty]] (P), which must
// be a fully populated Property Descriptor or Undefined.
assert(false, "assert_readonly", description,
"Object.getOwnPropertyDescriptor must return a fully populated property descriptor");
}
}
expose_assert(assert_readonly, "assert_readonly");

Expand Down

0 comments on commit 877d18d

Please sign in to comment.