Last Comment Bug 681615 - Move a couple of tests from content/html/content/test to content/html/content/test/forms
: Move a couple of tests from content/html/content/test to content/html/content...
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: DOM: Core & HTML (show other bugs)
: Trunk
: All All
: -- normal (vote)
: mozilla9
Assigned To: :Ms2ger
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2011-08-24 05:11 PDT by :Ms2ger
Modified: 2011-08-25 06:57 PDT (History)
0 users
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Patch v1 (6.22 KB, patch)
2011-08-24 05:11 PDT, :Ms2ger
mounir: review+
Details | Diff | Review
Patch v2 (9.78 KB, patch)
2011-08-24 07:34 PDT, :Ms2ger
mounir: review+
Details | Diff | Review

Description :Ms2ger 2011-08-24 05:11:48 PDT
Created attachment 555366 [details] [diff] [review]
Patch v1

I ran into those yesterday; I thought you might this.
Comment 1 Mounir Lamouri (:mounir) 2011-08-24 06:33:34 PDT
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.
Comment 2 :Ms2ger 2011-08-24 07:34:42 PDT
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?
Comment 3 Mounir Lamouri (:mounir) 2011-08-24 08:41:43 PDT
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
Comment 4 :Ms2ger 2011-08-24 11:45:40 PDT
No, that's just because it's an IDL attribute, nothing to do with reflection.

Note You need to log in before you can comment on or make changes to this bug.