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

element.getAttributeNames() spec doesn't match wpt and browsers #985

Closed
josepharhar opened this issue Jun 8, 2021 · 7 comments
Closed

Comments

@josepharhar
Copy link
Contributor

The spec for element.getAttributeNames says that it returns qualified names, which means that attribute namespaces should be included in the name. However, we can see that in chrome, firefox, and safari, attribute namespaces are not included in the output of getAttributeNames, and this behavior is actually tested in WPT.

const element = document.createElement('div');
element.setAttribute('nameone', 'valueone');
element.setAttributeNS('namespace', 'nametwo', 'valuetwo');
element.getAttributeNames(); // returns ['nameone', 'nametwo'] instead of ['nameone', 'namespace:nametwo']

Should we change the spec to say that getAttributeNames returns local names instead of qualified names? Or did I read something wrong?

(this came up in this chrome bug)

@sideshowbarker
Copy link
Contributor

sideshowbarker commented Jun 8, 2021

Thanks for raising this. Note that we also have a related MDN PR, from @trusktr, at mdn/content#5715.

@Ms2ger
Copy link
Member

Ms2ger commented Jun 8, 2021

The namespace "URL" is not included in the qualified name, only the namespace prefix is. There is no namespace prefix in

element.setAttributeNS('namespace', 'nametwo', 'valuetwo');

There is in the wpt test:

  el.setAttributeNS("dummy2", "dummy:foo", "bar");
  assert_equals(el.getAttributeNames()[3], "dummy:foo");

... so I think you just misread.

@sideshowbarker
Copy link
Contributor

Thanks for raising this. Note that we also have a related MDN PR, from @trusktr, at mdn/content#5715.

Since I’ve already mentioned that MDN PR here, I guess I should follow up that mention by saying that after @Ms2ger set me straight about the actual spec requirements, I’ve subsequently posted a review of that PR which attempts to bring it in conformance with what the spec says.

@josepharhar
Copy link
Contributor Author

Thanks for the explanation @Ms2ger!

@trusktr
Copy link

trusktr commented Jul 12, 2021

Can the API be fixed? Or will it forever be ambiguous?

@domenic
Copy link
Member

domenic commented Jul 12, 2021

There's nothing ambiguous about the API; it's well-specified. There was some confusion about what "qualified name" means but it was clarified on this thread (by referencing the spec).

@sideshowbarker
Copy link
Contributor

There's nothing ambiguous about the API; it's well-specified. There was some confusion about what "qualified name" means but it was clarified on this thread (by referencing the spec).

As somebody who was initially confused by this, at this point I can see that my confusion did not arise from the spec — which I think is quite clear. Instead, I can see now what caused my confusion was the the code that was included in the MDN patch at mdn/content#5715. I made the mistake of reading that code with the assumption that it was actually demonstrating what it claimed to be. But in fact that code does not demonstrate what it claims. See mdn/content#5715 (review).

Specifically that code seems to have been written with two misunderstandings: first, that getAttributeNames() outputs namespace URIs for namespaced attributes, and (2) that element.setAttributeNS('http://example.com/lorem', 'foo', 'bar') is going to cause getAttributeNames() to emit something other than just foo — but:

element.setAttributeNS('http://example.com/lorem', 'foo', 'bar') isn’t actually setting a namespace prefix for the foo attribute (it’s setting only the namespace itself) — and therefore per-spec the expected output for both iterations of that loop is just foo bar

And I don’t see any ambiguity in the spec or the exposed API that would cause somebody to have those two misunderstandings.

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