The default bug view has changed. See this FAQ.

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

RESOLVED FIXED in mozilla9

Status

()

Core
DOM: Core & HTML
RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: Ms2ger, Assigned: Ms2ger)

Tracking

Trunk
mozilla9
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Assignee)

Description

6 years ago
Created attachment 555366 [details] [diff] [review]
Patch v1

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+
(Assignee)

Comment 2

6 years ago
Created attachment 555397 [details] [diff] [review]
Patch v2

(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+
(Assignee)

Comment 4

6 years ago
No, that's just because it's an IDL attribute, nothing to do with reflection.
(Assignee)

Comment 5

6 years ago
http://hg.mozilla.org/mozilla-central/rev/80db03383c34
Status: ASSIGNED → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla9
You need to log in before you can comment on or make changes to this bug.