Closed Bug 630889 Opened 9 years ago Closed 9 years ago

textarea.rows and textarea.cols should be unsigned long and limited to only non-negative numbers greater than zero

Categories

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

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla5

People

(Reporter: mounir, Assigned: mounir)

References

(Blocks 1 open bug, )

Details

(Keywords: html5)

Attachments

(1 file, 1 obsolete file)

No description provided.
Keywords: html5
Attached patch Patch v1 (obsolete) — Splinter Review
Assignee: nobody → mounir.lamouri
Status: NEW → ASSIGNED
Attachment #509395 - Flags: review?(Olli.Pettay)
Blocks: 631203
Attached patch Patch v1.1Splinter Review
Attachment #509395 - Attachment is obsolete: true
Attachment #509398 - Flags: review?(Olli.Pettay)
Attachment #509395 - Flags: review?(Olli.Pettay)
Do you happen to know why this change was made to HTML spec?
In HTML4 those attributes are long.

(unsigned long makes sense, but I just wonder the backwards in-compatible to the
interface.)
Comment on attachment 509398 [details] [diff] [review]
Patch v1.1

>+
>+/** Test for Bug 630889 **/
>+
>+var textarea = document.createElement('textarea');
>+reflectUnsignedInt(textarea, "rows", true, 2);
>+reflectUnsignedInt(textarea, "cols", true, 20);

Could you add tests for some negative values and for 0.
Attachment #509398 - Flags: review?(Olli.Pettay) → review+
(In reply to comment #3)
> Do you happen to know why this change was made to HTML spec?

This change has been made in current HTML(5) specs.

(In reply to comment #4)
> Comment on attachment 509398 [details] [diff] [review]
> Patch v1.1
> >+var textarea = document.createElement('textarea');
> >+reflectUnsignedInt(textarea, "rows", true, 2);
> >+reflectUnsignedInt(textarea, "cols", true, 20);
> 
> Could you add tests for some negative values and for 0.

reflectUnsignedInt tests some negative values and 0.
Whiteboard: [can land][post-2.0]
(In reply to comment #5)
> (In reply to comment #3)
> > Do you happen to know why this change was made to HTML spec?
> 
> This change has been made in current HTML(5) specs.
But why?


> reflectUnsignedInt tests some negative values and 0.
Ah, great.
(In reply to comment #6)
> (In reply to comment #5)
> > (In reply to comment #3)
> > > Do you happen to know why this change was made to HTML spec?
> > 
> > This change has been made in current HTML(5) specs.
> But why?

Sorry, I thought you asked if it was in the current specs. I should be more careful.
I guess it has been made because it's more sane that way. I checked and Webkit seems to have it already so we can assume it will be safe (even if it's a post 2.0 change).
My bad, it doesn't throw an exception on Webkit but on Internet Explorer (at least 9).
Pushed:
http://hg.mozilla.org/mozilla-central/rev/4bb025590819

(IE6 has the same behavior FWIW)
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Whiteboard: [can land][post-2.0]
Target Milestone: --- → mozilla2.2
You need to log in before you can comment on or make changes to this bug.