[WebIDL] `window.CSS` should be non-callable object with correct Symbol.toStringTag

• Component: Bindings
• Status: RESOLVED FIXED

[Philip Jägenstedt]

Unlike interface objects, Web IDL namespace objects should be plain objects, not functions. Confirmed with the spec editors here:
https://github.com/heycam/webidl/issues/949

There are currently 3 namespace objects on the web: CSS, console and WebAssembly. CSS is currently a function in WebKit, and there's a failing idlharness.js test because of this, called 'CSS namespace: typeof is "object"':
https://wpt.fyi/results/css/cssom/idlharness.html?q=safari%3Afail&run_id=5750044512747520&run_id=5673650978029568&run_id=5657341938630656

However, the same test for console is passing in all browsers:
https://wpt.fyi/results/console/idlharness.any.html?run_id=5750044512747520&run_id=5673650978029568&run_id=5657341938630656

I can't find the test for WebAssembly, but it's an object as it should be.

[Radar WebKit Bug Importer]

<rdar://problem/73681411>

[Alexey Shvayka]

Created attachment 424542 [details]
Patch

[EWS Watchlist]

This patch modifies the imported WPT tests. Please ensure that any changes on the tests (not coming from a WPT import) are exported to WPT. Please see https://trac.webkit.org/wiki/WPTExportProcess

[Alexey Shvayka]

(In reply to Philip Jägenstedt from comment #0)

Thank you for raising the issue and creating the bug!

> I can't find the test for WebAssembly, but it's an object as it should be.

It is covered by https://wpt.fyi/results/wasm/jsapi/idlharness.any.html

[Sam Weinig]

Comment on attachment 424542 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=424542&action=review

Awesome work. Please add a new bindings test for namespace and partial namespace, so we catch regressions quickly if they happen.

> Source/WebCore/bindings/scripts/IDLParser.pm:316
> +sub convertNamespaceToInterface

This could use a comment explaining why it exists.

Ideally, we would keep namespace objects as IDLNamespace all the way through generation, but I gather that would require a lot of refactoring for little gain at this point.

[Alexey Shvayka]

Created attachment 427181 [details]
Patch for landing

[Alexey Shvayka]

(In reply to Sam Weinig from comment #5)
> Comment on attachment 424542 [details]
> Patch

I appreciate the review; thank you, Sam!

> Ideally, we would keep namespace objects as IDLNamespace all the way through
> generation, but I gather that would require a lot of refactoring for little
> gain at this point.

Right, that would be an enormous change just for `window.CSS`.

(In reply to Alexey Shvayka from comment #6)
> Created attachment 427181 [details]
> Patch for landing

Added a comment and bindings test for namespace / partial namespace.

[EWS]

Committed r276656 (237083@main): <https://commits.webkit.org/237083@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 427181 [details].

[Sam Weinig]

(In reply to Alexey Shvayka from comment #7)
> (In reply to Sam Weinig from comment #5)
> 
> Right, that would be an enormous change just for `window.CSS`.

Do any other specs (e.g. ones that maybe we don't implement in WebKit) use "namespace" yet?

[Alexey Shvayka]

(In reply to Sam Weinig from comment #9)
> Do any other specs (e.g. ones that maybe we don't implement in WebKit) use "namespace" yet?

According to comment #0 and quick search in WPT & CSSWG drafts: no (apart from `console` and `WebAssembly`, which aren't processed by code generator).
There are more specs with `partial namespace CSS` beyond CSS Houdini though (like https://github.com/w3c/csswg-drafts/blob/b4eb312fc9de41f31952d4a4d922f4c9b7cf51c9/css-images-4/Overview.bs#L1013).

To be certain, https://github.com/w3c/browser-specs could be scraped for all WebIDL definitions out there.

On a bright side, https://bugs.webkit.org/show_bug.cgi?id=158557 will remove 2 checks for namespace objects, and I have an idea how to simplify it further (remove more checks around GenerateConstructorHelperMethods).