Closed Bug 595449 Opened 12 years ago Closed 12 years ago

Implement elements IDL attribute for HTMLFieldsetElement

Categories

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

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla2.0b7

People

(Reporter: mounir, Assigned: mounir)

References

Details

(Keywords: dev-doc-complete, html5)

Attachments

(1 file, 1 obsolete file)

fieldset.elements should return an HTMLFormControlsCollection rooted to the fieldset filtering listed controls.
Depends on: 595543
Attached patch Patch v1 (obsolete) — Splinter Review
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).
Assignee: nobody → mounir.lamouri
Status: NEW → ASSIGNED
Attachment #474404 - Flags: review?(jonas)
Attached patch Patch v1Splinter Review
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.
Attachment #474404 - Attachment is obsolete: true
Attachment #474414 - Flags: review?(jonas)
Attachment #474404 - Flags: review?(jonas)
No longer depends on: 595543
Summary: Implement elements IDL attribute for HTMLFieldset → Implement elements IDL attribute for HTMLFieldsetElement
Comment on attachment 474414 [details] [diff] [review]
Patch v1

Moving the review to jst.
Attachment #474414 - Flags: review?(jonas) → review?(jst)
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).
Attachment #474414 - Flags: review?(jst)
Attachment #474414 - Flags: review+
Attachment #474414 - Flags: approval2.0+
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.
Pushed:
http://hg.mozilla.org/mozilla-central/rev/275d86ec7eee
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla2.0b7
Depends on: 613027
You need to log in before you can comment on or make changes to this bug.