[whatwg/webidl] Fix JSON type filtering in default toJSON operations (PR #1657)

Check an attribute's declared type before running its getter steps, instead of checking the returned value after invoking the getter. Omit non-JSON attributes without invoking their getters, matching existing browser binding generators.

Include enumeration types among JSON types, matching serialization of enum-valued attributes such as type on PerformanceNavigationTiming instances.

<!--
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 you submit this PR, and each time you edit this comment (including checking a checkbox through the UI!), PR Preview will run and update it. As such make any edits in one go and only after PR Preview has run.

If you think your PR is ready to land, please double-check that the build is passing and the checklist is complete before pinging.
-->

- [x] At least two implementers are interested (and none opposed):
    - Firefox checks `m.type.isJSONType()` **before generating the getter call**. [Codegen.py](https://github.com/mozilla-firefox/firefox/blob/main/dom/bindings/Codegen.py#L3499)
    - WebKit likewise skips non-JSON attributes before generating their getters. [CodeGeneratorJS.pm](https://github.com/WebKit/WebKit/blob/main/Source/WebCore/bindings/scripts/CodeGeneratorJS.pm#L6773)
    - Chromium’s handwritten LCP serializer reads its included fields and never reads `element`. [largest_contentful_paint.cc](https://github.com/chromium/chromium/blob/main/third_party/blink/renderer/core/timing/largest_contentful_paint.cc#L51)
- [x] [Tests](https://github.com/web-platform-tests/wpt) are written and can be reviewed and commented upon at:
   * Getter rearrangement is not really testable as existing getter steps for default toJSON()s don't have side effects.
   * [idlharness already allows enum types](https://github.com/web-platform-tests/wpt/blob/76fdb39425f8319118cba6c0c961e1d835c2879f/resources/idlharness.js#L579)
- [x] [Implementation bugs](https://github.com/whatwg/meta/blob/main/MAINTAINERS.md#handling-pull-requests) are filed:
   * Chromium: N/A
   * Gecko: N/A
   * WebKit: N/A
   * Deno: N/A?
   * Node.js: N/A?
   * webidl2.js: N/A
   * widlparser: N/A
- [x] [MDN issue](https://github.com/whatwg/meta/blob/main/MAINTAINERS.md#handling-pull-requests) is filed: Not needed
- [x] 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.)

You can view, comment on, or merge this pull request online at:

  https://github.com/whatwg/webidl/pull/1657

-- Commit Summary --

  * Fix JSON type filtering in default toJSON operations

-- File Changes --

    M index.bs (4)

-- Patch Links --

https://github.com/whatwg/webidl/pull/1657.patch
https://github.com/whatwg/webidl/pull/1657.diff

-- 
Reply to this email directly or view it on GitHub:
https://github.com/whatwg/webidl/pull/1657
You are receiving this because you are subscribed to this thread.

Message ID: <whatwg/webidl/pull/1657@github.com>