1013316 - Implement HTMLAllCollection::GetSupportedNames

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect)

Comment 1

[Boris Zbarsky [:bzbarsky]]

Attachment: Implement GetSupportedNames on HTMLAllCollection

This also fixes the GetSupportedNames on nsContentList HTMLCollections to follow the spec.

Comment 2

[Boris Zbarsky [:bzbarsky]]

Attachment: Implement GetSupportedNames on HTMLAllCollection

This also fixes the GetSupportedNames on nsContentList HTMLCollections to follow the spec.

Comment 3

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 8425963 [details] [diff] [review]
Implement GetSupportedNames on HTMLAllCollection


> static bool
>+IsAllNamedElement(nsIContent* aContent)
>+{
>+  nsIAtom* tag = aContent->Tag();
>+  return
>+    tag == nsGkAtoms::a ||
>+    tag == nsGkAtoms::applet ||
>+    tag == nsGkAtoms::button ||
>+    tag == nsGkAtoms::embed ||
>+    tag == nsGkAtoms::form ||
>+    tag == nsGkAtoms::iframe ||
>+    tag == nsGkAtoms::img ||
>+    tag == nsGkAtoms::input ||
>+    tag == nsGkAtoms::map ||
>+    tag == nsGkAtoms::meta ||
>+    tag == nsGkAtoms::object ||
>+    tag == nsGkAtoms::select ||
>+    tag == nsGkAtoms::textarea;


This list should have also frame and frameset

>+    nsGenericHTMLElement* el = nsGenericHTMLElement::FromContent(content);
>+    if (el) {
>+      // Note: nsINode::HasName means the name is exposed on the document,
>+      // which is false for options, so we don't check it here.
>+      const nsAttrValue* val = el->GetParsedAttr(nsGkAtoms::name);
>+      if (val && val->Type() == nsAttrValue::eAtom &&
>+          IsAllNamedElement(content)) {
Not sure which is faster here. GetParsedAttr or IsAllNamedElement. 
I guess this is fine.

Comment 4

[Boris Zbarsky [:bzbarsky]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/c0cf2e5d7c56

Comment 5

[Carsten Book [:Tomcat]]

Hi Boris, sorry had to backout this change in https://tbpl.mozilla.org/?tree=Mozilla-Inbound&rev=919fcf9baa00 since one of this 2 csets caused test failures like https://tbpl.mozilla.org/php/getParsedLog.php?id=40160645&tree=Mozilla-Inbound on all platforms

Comment 6

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

(In reply to Olli Pettay [:smaug] from comment #3)
> Comment on attachment 8425963 [details] [diff] [review]
> Implement GetSupportedNames on HTMLAllCollection
> 
> 
> > static bool
> >+IsAllNamedElement(nsIContent* aContent)
> >+{
> >+  nsIAtom* tag = aContent->Tag();
> >+  return
> >+    tag == nsGkAtoms::a ||
> >+    tag == nsGkAtoms::applet ||
> >+    tag == nsGkAtoms::button ||
> >+    tag == nsGkAtoms::embed ||
> >+    tag == nsGkAtoms::form ||
> >+    tag == nsGkAtoms::iframe ||
> >+    tag == nsGkAtoms::img ||
> >+    tag == nsGkAtoms::input ||
> >+    tag == nsGkAtoms::map ||
> >+    tag == nsGkAtoms::meta ||
> >+    tag == nsGkAtoms::object ||
> >+    tag == nsGkAtoms::select ||
> >+    tag == nsGkAtoms::textarea;
> 
> 
> This list should have also frame and frameset

content/html/document/test/test_documentAll.html seems to have disagreed.

Comment 7

[Boris Zbarsky [:bzbarsky]]

Gah, that's what I get for addressing review comments after the try run.  ;)

> content/html/document/test/test_documentAll.html seems to have disagreed.

It also disagrees with the spec, so it's just wrong.

Comment 8

[Boris Zbarsky [:bzbarsky]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/8ceaed67eda4 with that test fixed.

Comment 9

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/8ceaed67eda4