Closed Bug 595070 Opened 9 years ago Closed 9 years ago

HTMLInputElement list attribute has no DOM tests

Categories

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

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla2.0b7

People

(Reporter: mounir, Assigned: mounir)

References

Details

Attachments

(1 file, 1 obsolete file)

Part 1 patch in bug 556007 introduce input.list but this has not been tested. DOM tests might be quite easy to write. I will probably do that after beta6 or beta7. David, if you want to write them feel free to do it, just let me know.
I don't think I'll get to this any time soon.  If you haven't done it by November or so then I'll do it.
I found bug 603586 while writing the tests and another bug with id observer in document :/
Assignee: nobody → mounir.lamouri
Status: NEW → ASSIGNED
Depends on: 603586
Attached patch Tests (obsolete) — Splinter Review
Attachment #482508 - Flags: review?(jonas)
I've open bug 603591 for the second issue I've found (it's not an issue with .list).
Comment on attachment 482508 [details] [diff] [review]
Tests

This doesn't seem to test @list at all? Wrong patch?
Attachment #482508 - Flags: review?(jonas) → review-
Attached patch TestsSplinter Review
Sorry for attaching the wrong patch.
Attachment #482508 - Attachment is obsolete: true
Attachment #483922 - Flags: review?(jonas)
Comment on attachment 483922 [details] [diff] [review]
Tests

>+var tests = [ test1, test2, test3, test4, test5, test6, test7, test8, test9,
>+              test10, test11, test12, test13, test14, test15, test16, test17 ];
>+
>+test0();

Isn't test0 testing the same thing as test1 is? If you want to leave both in that's fine with me though.

>+for each (var test in tests) {
>+  var content = document.getElementById('content');
>+
>+  // Clean-up.
>+  while (content.firstChild) {
>+    content.removeChild(content.firstChild);
>+  }

A faster way to do this is: content.textContent = "";

r/a=me (though doesn't really need approval as it's NPOTB)
Attachment #483922 - Flags: review?(jonas)
Attachment #483922 - Flags: review+
Attachment #483922 - Flags: approval2.0+
(In reply to comment #7)
> Comment on attachment 483922 [details] [diff] [review]
> Tests
> 
> >+var tests = [ test1, test2, test3, test4, test5, test6, test7, test8, test9,
> >+              test10, test11, test12, test13, test14, test15, test16, test17 ];
> >+
> >+test0();
> 
> Isn't test0 testing the same thing as test1 is? If you want to leave both in
> that's fine with me though.

Indeed.

> >+for each (var test in tests) {
> >+  var content = document.getElementById('content');
> >+
> >+  // Clean-up.
> >+  while (content.firstChild) {
> >+    content.removeChild(content.firstChild);
> >+  }
> 
> A faster way to do this is: content.textContent = "";

Ok :)

> r/a=me (though doesn't really need approval as it's NPOTB)

Actually, it's a=tests.
Pushed:
http://hg.mozilla.org/mozilla-central/rev/f218d3480d61
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla2.0b8
Target Milestone: mozilla2.0b8 → mozilla2.0b7
You need to log in before you can comment on or make changes to this bug.