336331 - fix toDataURL as per spec

Status: RESOLVED FIXED

(Core :: Graphics: Canvas2D, defect)

Description

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

Update our canvas toDataURL impl to match that of the spec, and expose it to content with the right security checks.

Comment 1

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

Attachment: canvas-todataurl-fixes.patch

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.

Comment 2

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

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 3

[Robert O'Callahan (:roc) (email my personal email if necessary)]

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...

Comment 4

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

(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.

Comment 5

[cls]

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*)'

Comment 6

[cls]

Attachment: use NS_REINTERPRET_CAST to fix mingw bustage

Comment 7

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

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?

Comment 8

[cls]

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.