Implement elements IDL attribute for HTMLFieldsetElement

RESOLVED FIXED in mozilla2.0b7

Status

()

defect
RESOLVED FIXED
9 years ago
9 years ago

People

(Reporter: mounir, Assigned: mounir)

Tracking

({dev-doc-complete, html5})

Trunk
mozilla2.0b7
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

Assignee

Description

9 years ago
fieldset.elements should return an HTMLFormControlsCollection rooted to the fieldset filtering listed controls.
Assignee

Updated

9 years ago
Depends on: 595543
Assignee

Comment 1

9 years ago
Posted 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)
Assignee

Comment 2

9 years ago
Posted 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)
Assignee

Updated

9 years ago
No longer depends on: 595543
Assignee

Updated

9 years ago
Summary: Implement elements IDL attribute for HTMLFieldset → Implement elements IDL attribute for HTMLFieldsetElement
Assignee

Comment 3

9 years ago
Comment on attachment 474414 [details] [diff] [review]
Patch v1

Moving the review to jst.
Attachment #474414 - Flags: review?(jonas) → review?(jst)
Assignee

Comment 4

9 years ago
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.
Assignee

Comment 6

9 years ago
Pushed:
http://hg.mozilla.org/mozilla-central/rev/275d86ec7eee
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla2.0b7
Assignee

Updated

9 years ago
Depends on: 613027
You need to log in before you can comment on or make changes to this bug.