The default bug view has changed. See this FAQ.

canvas.width and canvas.height should be reflected as unsigned int

RESOLVED FIXED in mozilla5

Status

()

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

People

(Reporter: mounir, Assigned: mounir)

Tracking

(Blocks: 2 bugs, {dev-doc-complete, html5})

Trunk
mozilla5
dev-doc-complete, html5
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Assignee)

Description

7 years ago
Those attributes are declared as long instead of unsigned long in the IDL and the reflection isn't correct.
(Assignee)

Updated

7 years ago
Depends on: 610214
(Assignee)

Comment 1

7 years ago
Created attachment 488745 [details] [diff] [review]
Patch v1

Should we push that in Gecko 2.0 even if it changes the IDL?
Assignee: nobody → mounir.lamouri
Status: NEW → ASSIGNED
Attachment #488745 - Flags: review?(jst)
Attachment #488745 - Flags: approval2.0?
Attachment #488745 - Flags: approval2.0? → approval2.0-
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.
(Assignee)

Comment 3

7 years ago
(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).
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 on attachment 488745 [details] [diff] [review]
Patch v1

r=jst, but yeah, let's land this after branching.
Attachment #488745 - Flags: review?(jst) → review+
(Assignee)

Updated

6 years ago
Duplicate of this bug: 622783
Surely you need changes to test_canvas.html too?

Updated

6 years ago
Blocks: 622842
(Assignee)

Updated

6 years ago
Whiteboard: [can land][post-2.0]
Is this something to consider for Firefox 5?
Imo, yes.
(Assignee)

Comment 10

6 years ago
Pushed:
http://hg.mozilla.org/mozilla-central/rev/873390caf174
Status: ASSIGNED → RESOLVED
Last Resolved: 6 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Whiteboard: [can land][post-2.0]
Target Milestone: --- → mozilla2.2
Updated documentation:

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

And mentioned on Firefox 5 for developers.
Keywords: dev-doc-needed → dev-doc-complete

Updated

6 years ago
Duplicate of this bug: 581225
You need to log in before you can comment on or make changes to this bug.