Closed Bug 681615 Opened 8 years ago Closed 8 years ago

Move a couple of tests from content/html/content/test to content/html/content/test/forms

Categories

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

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla9

People

(Reporter: Ms2ger, Assigned: Ms2ger)

Details

Attachments

(1 file, 1 obsolete file)

Attached patch Patch v1 (obsolete) — Splinter Review
I ran into those yesterday; I thought you might this.
Attachment #555366 - Flags: review?(mounir)
Comment on attachment 555366 [details] [diff] [review]
Patch v1

Review of attachment 555366 [details] [diff] [review]:
-----------------------------------------------------------------

r=mounir, if you fix the things I pointed or open follow-ups.

::: content/html/content/test/forms/Makefile.in
@@ +60,5 @@
>  		test_output_element.html \
>  		test_button_attributes_reflection.html \
>  		test_textarea_attributes_reflection.html \
> +		test_validation.html \
> +		test_validation_tooLong.html \

I don't like those tests (yes, I wrote them...). I don't think there is anything we could do about them except review them and see what could be improved/fixed.
BTW, why test_validation_tooLong.html and not test_maxlength_attribute.html?

@@ +62,5 @@
>  		test_textarea_attributes_reflection.html \
> +		test_validation.html \
> +		test_validation_tooLong.html \
> +		test_datalist_options_attribute-1.html \
> +		test_datalist_options_attribute-2.html \

Could you merge them?
Maybe we should make sure the attribute reflection part is in another test too and we should name the test "test_datalist_element.html" because I don't see anything else to test.
Attachment #555366 - Flags: review?(mounir) → review+
Attached patch Patch v2Splinter Review
(In reply to Mounir Lamouri (:volkmar) from comment #1)
> Comment on attachment 555366 [details] [diff] [review]
> Patch v1
> 
> Review of attachment 555366 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> r=mounir, if you fix the things I pointed or open follow-ups.
> 
> ::: content/html/content/test/forms/Makefile.in
> @@ +60,5 @@
> >  		test_output_element.html \
> >  		test_button_attributes_reflection.html \
> >  		test_textarea_attributes_reflection.html \
> > +		test_validation.html \
> > +		test_validation_tooLong.html \
> 
> I don't like those tests (yes, I wrote them...). I don't think there is
> anything we could do about them except review them and see what could be
> improved/fixed.

Bug 681642

> BTW, why test_validation_tooLong.html and not test_maxlength_attribute.html?

Fixed

> @@ +62,5 @@
> >  		test_textarea_attributes_reflection.html \
> > +		test_validation.html \
> > +		test_validation_tooLong.html \
> > +		test_datalist_options_attribute-1.html \
> > +		test_datalist_options_attribute-2.html \
> 
> Could you merge them?

Sure.

> Maybe we should make sure the attribute reflection part is in another test
> too and we should name the test "test_datalist_element.html" because I don't
> see anything else to test.

Which attribute reflection part?
Attachment #555366 - Attachment is obsolete: true
Attachment #555397 - Flags: review?(mounir)
Comment on attachment 555397 [details] [diff] [review]
Patch v2

Review of attachment 555397 [details] [diff] [review]:
-----------------------------------------------------------------

function checkClassesAndAttributes checks if 'options' is an IDL attribute of HTMLDataListElement.
Do we want to keep that or move it to an attribute reflection test?

r=mounir
Attachment #555397 - Flags: review?(mounir) → review+
No, that's just because it's an IDL attribute, nothing to do with reflection.
http://hg.mozilla.org/mozilla-central/rev/80db03383c34
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla9
You need to log in before you can comment on or make changes to this bug.