whatwg / webidl

Web IDL Standard
https://webidl.spec.whatwg.org/
Other
410 stars 164 forks source link

legacy platform objects no longer override desc.[[Configurable]] #1418

Closed caitp closed 4 months ago

caitp commented 4 months ago

None of the code generation for named setters in any of the major browsers appears to honor Step 3. of 3.9.3. [DefineOwnProperty].

The test case in WPT (http://wpt.live/dom/collections/HTMLCollection-supported-property-names.html) already appears to assert that the descriptor is not forced to become [[Configurable]], and this test case is passing in most implementations.

I've done a bunch of detective work to justify this 2 line patch, so dumping all of that here. commit hashes are current as of writing this.

WebKit: https://github.com/WebKit/WebKit/blob/f288b9088d1b74b980c3b7a8d5add134f7689bdb/Source/WebCore/bindings/scripts/CodeGeneratorJS.pm#L1545-L1551

Chromium: https://source.chromium.org/chromium/chromium/src/+/b1387a25dd3e3ebb02ff33c935325a0d4d74c330:third_party/blink/renderer/bindings/scripts/bind_gen/interface.py;l=3484-3538

Gecko: https://github.com/mozilla/gecko-dev/blob/0b1d02b2cb5736511139cf0e40b318273e825899/dom/bindings/Codegen.py#L15261-L15301

With the [[GetOwnProperty]] algorithm, there are a few documented instances where this is intentionally ignored:

[[GetOwnProperty]] for Location objects: https://github.com/mozilla/gecko-dev/blob/0b1d02b2cb5736511139cf0e40b318273e825899/dom/bindings/Codegen.py#L16396-L16400 -- Per Gecko's passing the test, it does not appear to actually override the set descriptor in any way, at least not with regards to configurability.

In Chromium, similarly, it doesn't appear that any overriding occurs in LegacyPlatformObjecttGetOwnProperty: https://source.chromium.org/chromium/chromium/src/+/b1387a25dd3e3ebb02ff33c935325a0d4d74c330:third_party/blink/renderer/bindings/scripts/bind_gen/interface.py;l=3186-3202

Finally, in WebKit we also see no overriding behaviour for the descriptor attributes when the named property visibility algorithm returns false: https://github.com/WebKit/WebKit/blob/f288b9088d1b74b980c3b7a8d5add134f7689bdb/Source/WebCore/bindings/scripts/CodeGeneratorJS.pm#L983-L992

(See WHATWG Working Mode: Changes for more details.)


Preview | Diff

annevk commented 4 months ago

Given that browsers are interoperable and there's test coverage, and there's highly unlikely to be MDN coverage I think this is sufficient for merging.

I'll note in the final commit message that this fixes #669.