Bug 70088 – canvas getImageData should explain why it throws SECURITY_ERR

• Component: Canvas
• Status: RESOLVED FIXED

[Adam Barth]

Our poor error reporting confused Ulfar and made him sad.  Let's make him happy by adding a console message explaining why he can't read data from his canvas.

[Adam Barth]

Created attachment 110954 [details]
Patch

[WebKit Review Bot]

Comment on attachment 110954 [details]
Patch

Attachment 110954 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/10060424

New failing tests:
fast/canvas/svg-taint.html

[Adam Barth]

Created attachment 110956 [details]
Patch

[Darin Adler]

Comment on attachment 110956 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=110956&action=review

> Source/WebCore/html/canvas/CanvasRenderingContext2D.cpp:1762
> +        DEFINE_STATIC_LOCAL(String, consoleMessage, ("Unable to get image data from canvas because the canvas has been tainted by cross-origin data.\n"));

Do these messages really need newlines? I think this is an extra unneeded newline.

[Darin Adler]

Comment on attachment 110956 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=110956&action=review

> Source/WebCore/html/canvas/CanvasRenderingContext2D.cpp:59
> +#include "ScriptCallStack.h"

We should overload the addMessage function so callers that have no script context can use it without including ScriptCallStack.h and without supplying the number 1 and empty string just to say “I am not a script!”.

[Adam Barth]

(In reply to comment #5)
> (From update of attachment 110956 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=110956&action=review
> 
> > Source/WebCore/html/canvas/CanvasRenderingContext2D.cpp:59
> > +#include "ScriptCallStack.h"
> 
> We should overload the addMessage function so callers that have no script context can use it without including ScriptCallStack.h and without supplying the number 1 and empty string just to say “I am not a script!”.

Yes.  If it's ok with you, I'd like to do that in a separate patch so I can update all the callers at the same time.  Doing that in this patch would expand the scope of this patch significantly.

[Adam Barth]

Created attachment 111045 [details]
Patch for landing

[WebKit Review Bot]

Comment on attachment 111045 [details]
Patch for landing

Clearing flags on attachment: 111045

Committed r97509: <http://trac.webkit.org/changeset/97509>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.