Closed Bug 336331 Opened 14 years ago Closed 14 years ago

fix toDataURL as per spec

Categories

(Core :: Canvas: 2D, defect)

x86
All
defect
Not set

Tracking

()

RESOLVED FIXED

People

(Reporter: vlad, Assigned: vlad)

References

Details

Attachments

(2 files)

Update our canvas toDataURL impl to match that of the spec, and expose it to content with the right security checks.
This patch does a few toDataURL things:

- makes toDataURL accept no args, 1 arg, or 2 args (but 2 args only if caller is trusted)
- makes toDataURLAs [noscript], accepting 2 args.
- implements the missing bits for toDataURL if CAIRO_GFX

And also some stuff that I should have checked in before doing the todataurl, but I forgot:
- some MOZ_CAIRO_GFX/MOZILLA_1_8_BRANCH defines for branch-patch-parity
- fix for potential mlk in 333672, which is the GET_ARG macro.
Attachment #220579 - Flags: review?(roc)
Comment on attachment 220579 [details] [diff] [review]
canvas-todataurl-fixes.patch

>+
>+  int isCallerTrusted = -1; // -1 unknown, 0 no, 1 yes
>+

Ugh, ignore this line, pretend it's not there.
Comment on attachment 220579 [details] [diff] [review]
canvas-todataurl-fixes.patch

+  if ((mWriteOnly && argc < 2) || argc >= 2) {

if (mWriteOnly || argc >= 2) {

On trunk, we eventually need to change all this security check boilerplate to use nsContentUtils::IsCallerTrustedForRead.

Why the security checks in ToDataURLAs? It's not scriptable...
Attachment #220579 - Flags: superreview+
Attachment #220579 - Flags: review?(roc)
Attachment #220579 - Flags: review+
(In reply to comment #3)
> (From update of attachment 220579 [details] [diff] [review] [edit])
> +  if ((mWriteOnly && argc < 2) || argc >= 2) {
> 
> if (mWriteOnly || argc >= 2) {

Fixed

> On trunk, we eventually need to change all this security check boilerplate to
> use nsContentUtils::IsCallerTrustedForRead.
> 
> Why the security checks in ToDataURLAs? It's not scriptable...

Hrm.  No good reason; removed.  I forgot to remove them when I made ToDataURLAs [noscript].  I'm still going to keep the separate Impl method though, in case ToDataURLAs goes away at any point.
Status: NEW → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
This checkin caused the mingw build to break:

/usr/cls/src/moz/main/mozilla/content/html/content/src/nsHTMLCanvasElement.cpp: In member function `virtual nsresult nsHTMLCanvasElement::ToDataURL(nsAString_internal&)':
/usr/cls/src/moz/main/mozilla/content/html/content/src/nsHTMLCanvasElement.cpp:341: error: no matching function for call to `nsDependentString::nsDependentString(jschar*)'
Attachment #220602 - Flags: review?(vladimir)
Comment on attachment 220602 [details] [diff] [review]
use NS_REINTERPRET_CAST to fix mingw bustage

Ok, but why do none of the other gcc platforms need this?
Attachment #220602 - Flags: review?(vladimir) → review+
jschar is always a uint16.  On win32, PRUnichar is a wchar_t. On the other platforms, PRUnichar is a uint16.  mingw gcc doesn't seem to be able to cast between the two types, even when forcibly using -fshort-wchar.

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