Bug 97464 – Display a JS console message when setLineDash fails because of an invalid value

• Status: RESOLVED
• Resolution: WONTFIX
• Component: Canvas

[Justin Novosad]

When setLineDash receives a number sequence that contains a negative or non-finite value, it silently fails (no exception thrown), as prescribed by the spec.

A recent whatwg edit makes a non-normative suggestion that the UA could provide feedback to the developer by printing a console message that leaves a trace of the failure.
http://html5.org/tools/web-apps-tracker?from=7385&to=7386

[Justin Novosad]

Created attachment 168477 [details]
Patch

[Dean Jackson]

Comment on attachment 168477 [details]
Patch

I'm not sure I like this. Error messages annoy me unless I specifically asked for them. If we do this, then maybe we should have a special debug mode for all canvas operations.

[WebKit Review Bot]

Comment on attachment 168477 [details]
Patch

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

New failing tests:
fast/canvas/canvas-lineDash-invalid.html
platform/chromium/virtual/gpu/fast/canvas/canvas-lineDash-invalid.html

[WebKit Review Bot]

Comment on attachment 168477 [details]
Patch

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

New failing tests:
platform/chromium/virtual/gpu/fast/canvas/canvas-lineDash-invalid.html
fast/canvas/canvas-lineDash-invalid.html

[Build Bot]

Comment on attachment 168477 [details]
Patch

Attachment 168477 [details] did not pass mac-ews (mac):
Output: http://queues.webkit.org/results/14289092

New failing tests:
fast/canvas/canvas-lineDash-invalid.html

[Justin Novosad]

(In reply to comment #2)
> (From update of attachment 168477 [details])
> I'm not sure I like this. Error messages annoy me unless I specifically asked for them. If we do this, then maybe we should have a special debug mode for all canvas operations.

Isn't one specifically asking for error messages when the Dev console is visible?

Right now, the spec is kind of inconsistent in how invalid arguments are handled.  In many cases, a DOM exception is fired, in other cases, failures are silent.  The choice between the two seems arbitrary in many cases. It looks like newer APIs favor silent failures. Perhaps this is a backlash from annoyance with DOM exceptions (because they interrupt JS execution). Silent failures are at the other extreme.  Error messages seem like a reasonable compromise.

After thinking a little deeper about this, I think we need a general consensus around this and we should be consistent about whether or not to print error messages about invalid args across the entire canvas API (and possibly other APIs).

Putting this on hold for now.

[Dean Jackson]

(In reply to comment #6)
> (In reply to comment #2)
> > (From update of attachment 168477 [details] [details])
> > I'm not sure I like this. Error messages annoy me unless I specifically asked for them. If we do this, then maybe we should have a special debug mode for all canvas operations.
> 
> Isn't one specifically asking for error messages when the Dev console is visible?

Not necessarily, and possibly not in canvas code. Imagine you include some library in your page that has an error in line dash - it could spew to the console hiding errors you might be more interested in. Also, we don't always log CSS errors, another thing that it impossible to detect through exceptions.

I often open the dev console to execute code.

> Right now, the spec is kind of inconsistent in how invalid arguments are handled.  In many cases, a DOM exception is fired, in other cases, failures are silent.  The choice between the two seems arbitrary in many cases. It looks like newer APIs favor silent failures. Perhaps this is a backlash from annoyance with DOM exceptions (because they interrupt JS execution). Silent failures are at the other extreme.  Error messages seem like a reasonable compromise.
> 
> After thinking a little deeper about this, I think we need a general consensus around this and we should be consistent about whether or not to print error messages about invalid args across the entire canvas API (and possibly other APIs).
> 

That's what I was getting at. It would be nice to have some uniform way to display errors from canvas. e.g. WebGL has a way to query the context for errors.

[Darin Adler]

Comment on attachment 168477 [details]
Patch

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

Why are we doing this for line dash but not, for example, shadow color? Seems random.

> Source/WebCore/ChangeLog:9
> +        Test: fast/canvas/canvas-lineDash-invalid.html + manual validation
> +        of console message

We don’t have any way to regression-test console messages? That seems like something we should fix.

[Build Bot]

Comment on attachment 168477 [details]
Patch

Attachment 168477 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://queues.webkit.org/results/15925275

[WebKit Review Bot]

Comment on attachment 168477 [details]
Patch

Attachment 168477 [details] did not pass cr-linux-debug-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/16218711

[Zan Dobersek]

Comment on attachment 168477 [details]
Patch

The work on this bug has ceased, removing the r? flag on the patch.