595449 - Implement elements IDL attribute for HTMLFieldsetElement

Status: RESOLVED FIXED

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

Description

[Mounir Lamouri (:mounir)]

fieldset.elements should return an HTMLFormControlsCollection rooted to the fieldset filtering listed controls.

Comment 1

[Mounir Lamouri (:mounir)]

Attachment: Patch v1

Two things about this patch:
1. One test do not pass because of bug 595543.
2. This is not entirely following the specifications. Indeed, specifications ask for elements to be HTMLFormControlsCollection but elements is HTMLCollection here. HTMLFormControlsCollection is only changing .namedItem() behavior. Opera has .elements implemneted but not HTMLFormControlsCollection too.
If you think we should not ship with that, I will cut the bug in two so I still have mElements (which might be needed for fieldset.disbaled).

Comment 2

[Mounir Lamouri (:mounir)]

Attachment: Patch v1

I realized that bug 595543 wasn't so obvious. <label> is a form associated element (has .form and @form) but not a listed element... I've no idea why this has to be a form associated element.
So, the test is now fixed.

Comment 3

[Mounir Lamouri (:mounir)]

Comment on attachment 474414 [details] [diff] [review]
Patch v1

Moving the review to jst.

Comment 4

[Mounir Lamouri (:mounir)]

I spoke quickly with Jonas and he thinks we can ship fieldset.elements like that. I will add a test checking that fieldset.elements.foo returns the element with the name "foo" in it (this is working, just need to add a test).

Comment 5

[Johnny Stenback (:jst)]

Comment on attachment 474414 [details] [diff] [review]
Patch v1

@@ -50,17 +50,17 @@ class nsHTMLFieldSetElement : public nsG
 {
 public:
   using nsIConstraintValidation::GetValidationMessage;
 
   nsHTMLFieldSetElement(already_AddRefed<nsINodeInfo> aNodeInfo);
   virtual ~nsHTMLFieldSetElement();
 
   // nsISupports
-  NS_DECL_ISUPPORTS_INHERITED
+  NS_DECL_CYCLE_COLLECTING_ISUPPORTS

Sorry, jumped the gun there and forgot to add a comment here. This change is incorrect, it will add an mRefCnt member to this class instead of inheriting the existing one. So fix that before landing.

Comment 6

[Mounir Lamouri (:mounir)]

Pushed:
http://hg.mozilla.org/mozilla-central/rev/275d86ec7eee