Bug 159855 - Exceptions logged to the JS console should use toString()
Summary: Exceptions logged to the JS console should use toString()
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Brady Eidson
URL:
Keywords:
: 159845 159846 (view as bug list)
Depends on: 159822
Blocks:
  Show dependency tree / graph
 
Reported: 2016-07-16 12:43 PDT by Brady Eidson
Modified: 2016-07-17 12:14 PDT (History)
2 users (show)

See Also:

Attachments
Patch (6.69 KB, patch)
2016-07-17 10:14 PDT, Brady Eidson 		no flags Details | Formatted Diff | Diff
Patch (7.25 KB, patch)
2016-07-17 10:22 PDT, Brady Eidson 		darin: review+
Details | Formatted Diff | Diff
Show Obsolete (1) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Brady Eidson 2016-07-16 12:43:42 PDT 
Change how exceptions are logged in the JS console to not look redundant and silly.

This is needed with work on https://bugs.webkit.org/show_bug.cgi?id=159822, which is making exception messages more rich.

An example of how this work effects console.log can be found here - https://bugs.webkit.org/show_bug.cgi?id=159847#c2

CONSOLE MESSAGE: line 14: An invalid value was passed to an operation or assigned to an attribute.: An invalid value was passed to an operation or assigned to an attribute.

The code responsible is in JSDOMBinding.cpp:

void reportException(ExecState* exec, Exception* exception, CachedScript* cachedScript, ExceptionDetails* exceptionDetails)
...
        errorMessage = exceptionBase->message() + ": "  + exceptionBase->description();

For Exceptions that have adopted the "description as message" mode, the message and the description are the same.

This can all get cleaned up once the task represented by 159822 is complete, but for now we should do something a little different for the new exceptions vs the old ones.
Comment 1 Brady Eidson 2016-07-16 12:50:05 PDT 
Retitled:
Exceptions logged to the JS console should use toString()
Comment 2 Brady Eidson 2016-07-17 08:41:42 PDT 
*** Bug 159845 has been marked as a duplicate of this bug. ***
Comment 3 Brady Eidson 2016-07-17 08:41:46 PDT 
*** Bug 159846 has been marked as a duplicate of this bug. ***
Comment 4 Darin Adler 2016-07-17 09:29:42 PDT 
When we change this to use toString, I think we possibly want to use a variant of toString that does not run arbitrary code.
Comment 5 Brady Eidson 2016-07-17 10:14:55 PDT 
Created attachment 283866 [details]
Patch
Comment 6 Brady Eidson 2016-07-17 10:15:43 PDT 
(In reply to comment #4)
> When we change this to use toString, I think we possibly want to use a
> variant of toString that does not run arbitrary code.

Okay.
Comment 7 Brady Eidson 2016-07-17 10:19:16 PDT 
(In reply to comment #6)
> (In reply to comment #4)
> > When we change this to use toString, I think we possibly want to use a
> > variant of toString that does not run arbitrary code.
> 
> Okay.

Seems useful to not initialize a "toString" message when the ExceptionBase is made, as it's not always needed.

So toString() will still run code, but cache the result.
Comment 8 Brady Eidson 2016-07-17 10:22:05 PDT 
Created attachment 283867 [details]
Patch
Comment 9 Darin Adler 2016-07-17 10:57:13 PDT 
Comment on attachment 283867 [details]
Patch

This is fine. When I said not run "arbitrary code", I meant specifically that it should not run arbitrary JavaScript code, not C++ code.
Comment 10 Brady Eidson 2016-07-17 12:12:29 PDT 
(In reply to comment #9)
> Comment on attachment 283867 [details]
> Patch
> 
> This is fine. When I said not run "arbitrary code", I meant specifically
> that it should not run arbitrary JavaScript code, not C++ code.

Ah! Yah, we're definitely fine there.
Comment 11 Brady Eidson 2016-07-17 12:14:08 PDT 
https://trac.webkit.org/changeset/203334