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

attachShadow must throw a TypeError when mode is omitted #311

Closed
rniwa opened this issue Sep 3, 2015 · 7 comments
Closed

attachShadow must throw a TypeError when mode is omitted #311

rniwa opened this issue Sep 3, 2015 · 7 comments

Comments

@rniwa
Copy link
Collaborator

rniwa commented Sep 3, 2015

attachShadow must throw a TypeError when "mode" is omitted in ShadowRootInit as previously agreed in F2F.

@rniwa
Copy link
Collaborator Author

rniwa commented Sep 3, 2015

We should also throw when the value of mode doesn't match either "open" or "closed".

@domenic
Copy link
Collaborator

domenic commented Sep 3, 2015

I think the correct way to do this is to make mode required in the definition of ShadowRootInit. Should be a one-line fix.

@domenic
Copy link
Collaborator

domenic commented Sep 3, 2015

Actually the spec already takes care of this. When you convert a JS value to a ShadowRootMode, per IDL, first you do a ToString, and if it does not match "open" or "closed" (including if it is "undefined"), it will throw a TypeError.

@rniwa
Copy link
Collaborator Author

rniwa commented Sep 3, 2015

But it could still be omitted from the dictionary, right?

@domenic
Copy link
Collaborator

domenic commented Sep 3, 2015

Yeah, I just re-read, and I was right the first time. Without required, if it is undefined (e.g. via omission from the JS object that gets converted into an IDL dictionary), the conversion to ShadowRootMode will not take place.

So yeah, just add required and it'll work as expected.

@hayatoito
Copy link
Contributor

I thought it's required by default unless optional is specified. What's weird. :(
Where can I read the definition of required?
Is this the latest spec we should use? https://heycam.github.io/webidl/

@annevk
Copy link
Collaborator

annevk commented Sep 4, 2015

No, dictionary members are always optional unless prefixed with required. And yes, that is the latest IDL.

kojiishi pushed a commit to kojiishi/webcomponents that referenced this issue Sep 7, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants