Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Should namespace objects have "length" and "name" own properties? #949

Closed
foolip opened this issue Jan 21, 2021 · 8 comments
Closed

Should namespace objects have "length" and "name" own properties? #949

foolip opened this issue Jan 21, 2021 · 8 comments

Comments

@foolip
Copy link
Member

foolip commented Jan 21, 2021

I can't find anything in https://heycam.github.io/webidl/#namespace-object to suggest that a namespace object should have length or name properties, but looking at namespace objects in implementations the results are mixed:

  • CSS.length is 0 and CSS.name is "CSS" in Chrome, Firefox and Safari
  • console.length is 0 and console.name is "console" in Firefox, but both are undefined in Chrome and Safari
  • WebAssembly.length and WebAssembly.name are undefined in Chrome, Firefox and Safari

I don't think this has caused any problems, but it could be a future nuisance making it harder to introduce a new namespace which has a name member. (Wanting a length member seems less likely.)

With confirmation that these shouldn't be there, I could add negative tests for it.

@domenic
Copy link
Member

domenic commented Jan 21, 2021

They should not. This is due to incorrect implementations of them as functions instead of namespace objects. You could also test that typeof x === "object" (not "function").

@TimothyGu
Copy link
Member

It appears to be a bit more nuanced. On Chrome 88, CSS.length and CSS.name are both undefined, contrary to @foolip's findings. OTOH typeof CSS is "object" on both Chrome and Firefox here, though it's "function" on WebKit.

@foolip
Copy link
Member Author

foolip commented Jan 22, 2021

I was testing in Chrome 87. In Chrome Canary, I see that CSS.length and CSS.name are indeed both undefined. And typeof CSS has changed from "function" to "object". This was a recent fix, perhaps in https://chromium-review.googlesource.com/c/chromium/src/+/2463092. @yuki3 was that an intentional effect of that CL?

Regardless, sounds like we should have tests for each of the namespace objects to ensure they're objects, not functions. Checking that they don't have length and name is then a bit redundant, but maybe we should.

@foolip
Copy link
Member Author

foolip commented Jan 22, 2021

Turns out idlharness.js tests this. In https://wpt.fyi/results/css/cssom/idlharness.html?label=experimental&label=master&aligned, the 'CSS namespace: typeof is "object"' test fails in Safari. I'll file bugs.

@foolip
Copy link
Member Author

foolip commented Jan 22, 2021

OK, I've filed https://bugs.webkit.org/show_bug.cgi?id=220855.

@foolip foolip closed this as completed Jan 22, 2021
@yuki3
Copy link

yuki3 commented Jan 22, 2021

Yes, the change was intentional. https://chromium-review.googlesource.com/c/chromium/src/+/2434404 is the patch to make CSS an IDL namespace.

@saschanaz
Copy link
Member

I think Gecko does have a bug here, filed https://bugzilla.mozilla.org/show_bug.cgi?id=1688335

@foolip
Copy link
Member Author

foolip commented Jan 23, 2021

Thanks for confirming @yuki3, I didn't think to check if CSS was recently changes to a namespace and just assumed it was a bug with namespace objects generally.

And thanks @saschanaz for filing that Gecko bug, I forgot about console when I closed the issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

5 participants