Last Comment Bug 610212 - canvas.width and canvas.height should be reflected as unsigned int
: canvas.width and canvas.height should be reflected as unsigned int
Status: RESOLVED FIXED
: dev-doc-complete, html5
Product: Core
Classification: Components
Component: DOM: Core & HTML (show other bugs)
: Trunk
: All All
: -- normal (vote)
: mozilla5
Assigned To: Mounir Lamouri (:mounir)
:
Mentors:
: 581225 622783 (view as bug list)
Depends on: 601061 610214
Blocks: 586126 622842
  Show dependency treegraph
 
Reported: 2010-11-07 06:10 PST by Mounir Lamouri (:mounir)
Modified: 2011-10-23 02:42 PDT (History)
7 users (show)
mounir: in‑testsuite+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Patch v1 (4.50 KB, patch)
2010-11-07 06:21 PST, Mounir Lamouri (:mounir)
jst: review+
benjamin: approval2.0-
Details | Diff | Splinter Review

Description Mounir Lamouri (:mounir) 2010-11-07 06:10:05 PST
Those attributes are declared as long instead of unsigned long in the IDL and the reflection isn't correct.
Comment 1 Mounir Lamouri (:mounir) 2010-11-07 06:21:23 PST
Created attachment 488745 [details] [diff] [review]
Patch v1

Should we push that in Gecko 2.0 even if it changes the IDL?
Comment 2 Benjamin Smedberg [:bsmedberg] 2010-11-10 13:34:54 PST
Unless there's something not listed in the bug, I don't think this is worth breaking API freeze for. I know that several binary extensions use nsIDOMHTMLCanvasElement.
Comment 3 Mounir Lamouri (:mounir) 2010-11-10 17:35:38 PST
(In reply to comment #2)
> Unless there's something not listed in the bug, I don't think this is worth
> breaking API freeze for. I know that several binary extensions use
> nsIDOMHTMLCanvasElement.

We might be one of the last "modern browsers" to have canvas.width and canvas.height to not default correctly and we are failing some tests in HTML5 Test Suite because of that. But I agree I do not see a reason big enough to break compatibility.
In another hand, I really don't see how moving from long to unsigned long might break something (it would be insane to set a negative value to a width or a height).
Comment 4 Benjamin Smedberg [:bsmedberg] 2010-11-11 06:37:10 PST
I figured we'd never actually get negative values, so that's not my concern. We're already past API-freeze for Firefox 4, which means that IDL changes are not allowed except by very special exception.
Comment 5 Johnny Stenback (:jst, jst@mozilla.com) 2010-11-18 15:01:14 PST
Comment on attachment 488745 [details] [diff] [review]
Patch v1

r=jst, but yeah, let's land this after branching.
Comment 6 Mounir Lamouri (:mounir) 2011-01-04 02:58:49 PST
*** Bug 622783 has been marked as a duplicate of this bug. ***
Comment 7 :Ms2ger (⌚ UTC+1/+2) 2011-01-04 04:32:54 PST
Surely you need changes to test_canvas.html too?
Comment 8 :Ehsan Akhgari 2011-03-22 13:40:32 PDT
Is this something to consider for Firefox 5?
Comment 9 Boris Zbarsky [:bz] 2011-03-22 13:44:52 PDT
Imo, yes.
Comment 10 Mounir Lamouri (:mounir) 2011-03-25 07:46:11 PDT
Pushed:
http://hg.mozilla.org/mozilla-central/rev/873390caf174
Comment 11 Eric Shepherd [:sheppy] 2011-04-12 10:55:42 PDT
Updated documentation:

https://developer.mozilla.org/en/HTML/Element/canvas

And mentioned on Firefox 5 for developers.
Comment 12 :Ms2ger (⌚ UTC+1/+2) 2011-10-23 02:42:21 PDT
*** Bug 581225 has been marked as a duplicate of this bug. ***

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