- From: ⭐caitp⭐ <notifications@github.com>
- Date: Tue, 09 Jul 2024 18:57:08 -0700
- To: whatwg/webidl <webidl@noreply.github.com>
- Cc: Subscribed <subscribed@noreply.github.com>
- Message-ID: <whatwg/webidl/pull/1418@github.com>
None of the code generation for named setters in any of the major browsers appears to honor Step 3. of 3.9.3. [[DefineOwnProperty]]( https://webidl.spec.whatwg.org/#legacy-platform-object-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 4 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 <!-- Thank you for contributing to the Web IDL Standard! Please describe the change you are making and complete the checklist below if your change is not editorial. When editing this comment after the PR is created, check that PR-Preview doesn't overwrite your changes. If you think your PR is ready to land, please double-check that the build is passing and the checklist is complete before pinging. --> - [ ] At least two implementers are interested (and none opposed): * … * … - [ ] [Tests](https://github.com/web-platform-tests/wpt) are written and can be reviewed and commented upon at: * … <!-- If these tests are tentative, link a PR to make them non-tentative. --> - [ ] [Implementation bugs](https://github.com/whatwg/meta/blob/main/MAINTAINERS.md#handling-pull-requests) are filed: * Chromium: … * Gecko: … * WebKit: … * Deno: … * Node.js: … * webidl2.js: … * widlparser: … - [ ] [MDN issue](https://github.com/whatwg/meta/blob/main/MAINTAINERS.md#handling-pull-requests) is filed: … - [ ] The top of this comment includes a [clear commit message](https://github.com/whatwg/meta/blob/main/COMMITTING.md) to use. <!-- If you created this PR from a single commit, Github copied its message. Otherwise, you need to add a commit message yourself. --> (See [WHATWG Working Mode: Changes](https://whatwg.org/working-mode#changes) for more details.) (TODO: edit this comment to fit with the template -- I feel like I already included a lot of this information, still need to check up on mdn and stuff) <!-- This comment and the below content is programmatically generated. You may add a comma-separated list of anchors you'd like a direct link to below (e.g. #idl-serializers, #idl-sequence): Don't remove this comment or modify anything below this line. If you don't want a preview generated for this pull request, just replace the whole of this comment's content by "no preview" and remove what's below. --> *** <a href="https://whatpr.org/webidl/1418.html" title="Last updated on Jul 10, 2024, 1:56 AM UTC (1c4159a)">Preview</a> | <a href="https://whatpr.org/webidl/1418/1ce5eb5...1c4159a.html" title="Last updated on Jul 10, 2024, 1:56 AM UTC (1c4159a)">Diff</a> You can view, comment on, or merge this pull request online at: https://github.com/whatwg/webidl/pull/1418 -- Commit Summary -- * legacy platform objects no longer override desc.[[Configurable]] -- File Changes -- M index.bs (4) -- Patch Links -- https://github.com/whatwg/webidl/pull/1418.patch https://github.com/whatwg/webidl/pull/1418.diff -- Reply to this email directly or view it on GitHub: https://github.com/whatwg/webidl/pull/1418 You are receiving this because you are subscribed to this thread. Message ID: <whatwg/webidl/pull/1418@github.com>
Received on Wednesday, 10 July 2024 01:57:12 UTC