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

CustomElementRegistry.prototype.get() and case sensitivity #604

Closed
treshugart opened this issue Nov 10, 2016 · 11 comments
Closed

CustomElementRegistry.prototype.get() and case sensitivity #604

treshugart opened this issue Nov 10, 2016 · 11 comments

Comments

@treshugart
Copy link

I was hoping to get clarification on whether or not the name passed into customElements.get() is case-sensitive. In Chrome it is not, and in Safari Tech Preview it is. Depending on the outcome of this issue, I'll file a bug in the appropriate bug tracker. Thanks!

@rniwa
Copy link
Collaborator

rniwa commented Nov 10, 2016

As far as I can tell and implemented, it's not case-insensitive:
https://html.spec.whatwg.org/#dom-customelementregistry-get

When invoked, the get(name) method must run these steps:

  1. If this CustomElementRegistry contains an entry with name name, then return that entry's constructor.
  2. Otherwise, return undefined.

@annevk
Copy link
Collaborator

annevk commented Nov 10, 2016

Would be good to test, but yeah, I wonder how Chrome ended up not matching this.

@treshugart
Copy link
Author

As far as I can tell and implemented, it's not case-insensitive:

As a consumer of the APIs, I would assume case-insensitive because it would seem that's how other parts of the DOM API behave. For example querySelector() and setAttribute(). Furthermore, if someone were to register X-ELEMENT, and it still upgrades <x-element />, but when you try and get('x-element') it returns null, then that would seem odd.

@treshugart
Copy link
Author

Also here, as the library using this method, we don't know what case the consumer used and since tagName is always uppercase, we have no way of telling.

@treshugart
Copy link
Author

If we had something like #566, then we could get the exact casing with only a constructor reference.

@annevk
Copy link
Collaborator

annevk commented Nov 10, 2016

You should use localName, not tagName.

@rniwa
Copy link
Collaborator

rniwa commented Nov 10, 2016

Calling customElements.define('X-ELEMENT', ~) would throw we because we explicitly disallow uppercase ASCII letters.

By the way, setAttribute isn't case-insensitive. It simply converts the qualified name to lowercase letters: https://dom.spec.whatwg.org/#dom-element-setattribute so if you happen to have an attribute with uppercase letters (which is possible), then setAttribute would not override that attribute.

@treshugart
Copy link
Author

treshugart commented Nov 10, 2016

You should use localName, not tagName.

Yep, my bad. Fixed.

Calling customElements.define('X-ELEMENT', ~) would throw we because we explicitly disallow uppercase ASCII letters.

Awesome, thank you all for being so quick to respond and sorry for any ignorance on my part. So looks like a bug in Blink then, or should we just continue to be defensive like we are?

EDIT

I assume localName should return all lowercase in all browsers.

@rniwa
Copy link
Collaborator

rniwa commented Nov 10, 2016

localName can contain uppercase letters. e.g. document.createElementNS(null, 'aBc').localName

@annevk
Copy link
Collaborator

annevk commented Nov 10, 2016

@treshugart will you file a bug against Chrome? Would be good to mention it here. We should probably close this once that is done or maybe wait until there's test coverage.

@treshugart
Copy link
Author

Hmm, I went to reproduce a reduced test case and it seems to be working fine now. That's really odd because Chrome worked and toLowerCase() made Safari work. That was literally the only difference in the code. I'll close this for now. If I end up reproducing it I'll raise an issue and link to this.

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

3 participants