Bug 143623 – Web Inspector: Improve Console Message Formatting

• Status: RESOLVED
• Resolution: FIXED
• Component: Web Inspector

[Joseph Pecoraro]

* SUMMARY
Improve Console Message Formatting. Mostly focusing on handling of extra objects.

One of the goals of the new console design was to eliminate expanding multiple objects on the same line. In that way, a console.log/error/etc with multiple objects should be presented with each object on its own line. This eliminates the confusing and poor experience that comes with expanding/collapsing objects sharing the same line which causes everything to move when expanding.

The different cases for logging are and proposed UI are:

    1. console.log("message")
      => message

    2. console.log("format string message: %s", "foo")
      => format string message: foo

    3. console.log(valueOrLosslessObject)
      => {a: 1}

    4. console.log(lossyObject)
      => ▶︎ Object {a: 1, b: 2, c: 3, d: 4, e: 5, ...}

    6. console.log("message", valueOrLosslessObject)
      => message – {a: 1}

    7. console.log("message", lossyObject)
      => ▶︎ message – Object {a: 1, b: 2, c: 3, ...}

    b. (it is collapsed by default, so after expanding)
      => ▼ message
         • ▼ Object
            a: 1
            b: 2
            ...

    8. console.log("message", anyValue1, anyValue2)
      => ▼ message (2)
         • {a: 1}
         • ▶︎ Object {a: 1, b: 2, c: 3, d: 4, e: 5, ...}

    b. (it is expanded by default, so after collapsing)
      => ▶︎ message (2)

    9. console.log(anyValue1, anyValue2)
      => ▼ No message (2)
         • {a: 1}
         • ▶︎ Object {a: 1, b: 2, c: 3, d: 4, e: 5, ...}


* NOTES
- There is an initial version of what this would look like at: http://timothy.hatcher.name/console/
- Currently for console.error we show the backtrace / call stack after the objects. Those can still show up underneath these "extra objects" for now.

[Radar WebKit Bug Importer]

<rdar://problem/20507112>

[Joseph Pecoraro]

Created attachment 250542 [details]
[TEST] Test for different console log cases

[Joseph Pecoraro]

Created attachment 250543 [details]
[PATCH] Proposed Fix

[Joseph Pecoraro]

Comment on attachment 250543 [details]
[PATCH] Proposed Fix

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

> Source/WebInspectorUI/UserInterface/Views/ConsoleMessageView.css:106
> +    margin-left: 24px;

This should have been 20px.

[Timothy Hatcher]

Comment on attachment 250543 [details]
[PATCH] Proposed Fix

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

> Source/WebInspectorUI/UserInterface/Views/ConsoleMessageView.css:204
> +    color: hsl(0, 0%, 67%);

Might want to use the same color as .console-message-enclosed.

> Source/WebInspectorUI/UserInterface/Views/ConsoleMessageView.js:149
> +            this._boundExpandClickHandler = function() { this.toggle(); }.bind(this);

Though this is safer, it also makes an extra closure that isn't needed. Maybe JSC ignores that since there is not closure vars used?

> Source/WebInspectorUI/UserInterface/Views/ConsoleMessageView.js:338
> +        this._stackTraceElement = this._element.appendChild(document.createElement("ul"));
> +        this._stackTraceElement.classList.add("console-message-stack-trace-container");
> +        this._stackTraceElement.classList.add("console-message-text");
> +
> +        for (var callFrame of this._message.stackTrace) {
> +            var callFrameElement = this._stackTraceElement.appendChild(document.createElement("li"));
> +            callFrameElement.classList.add("console-message-stack-trace-call-frame");
> +            callFrameElement.textContent = callFrame.functionName || WebInspector.UIString("(anonymous function)");
> +            if (callFrame.url && !this._shouldHideURL(callFrame.url))
> +                callFrameElement.appendChild(this._linkifyCallFrame(callFrame));
>          }

We might want to clean this up with a custom UI with function icons and locations like the stack frame sidebar section. To unify how we show stack traces.

[Joseph Pecoraro]

(In reply to comment #5)
> Comment on attachment 250543 [details]
> [PATCH] Proposed Fix
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=250543&action=review
> 
> > Source/WebInspectorUI/UserInterface/Views/ConsoleMessageView.css:204
> > +    color: hsl(0, 0%, 67%);
> 
> Might want to use the same color as .console-message-enclosed.

Great catch! I actually just fixed all of these to be:

    color: rgba(0, 0, 0, 0.33);

> 
> > Source/WebInspectorUI/UserInterface/Views/ConsoleMessageView.js:149
> > +            this._boundExpandClickHandler = function() { this.toggle(); }.bind(this);
> 
> Though this is safer, it also makes an extra closure that isn't needed.
> Maybe JSC ignores that since there is not closure vars used?

Another great catch! I used to have more code in the handler but later moved it to expand. We should totally be able to make this "this.toggle".

> 
> > Source/WebInspectorUI/UserInterface/Views/ConsoleMessageView.js:338
> > +        this._stackTraceElement = this._element.appendChild(document.createElement("ul"));
> > +        this._stackTraceElement.classList.add("console-message-stack-trace-container");
> > +        this._stackTraceElement.classList.add("console-message-text");
> > +
> > +        for (var callFrame of this._message.stackTrace) {
> > +            var callFrameElement = this._stackTraceElement.appendChild(document.createElement("li"));
> > +            callFrameElement.classList.add("console-message-stack-trace-call-frame");
> > +            callFrameElement.textContent = callFrame.functionName || WebInspector.UIString("(anonymous function)");
> > +            if (callFrame.url && !this._shouldHideURL(callFrame.url))
> > +                callFrameElement.appendChild(this._linkifyCallFrame(callFrame));
> >          }
> 
> We might want to clean this up with a custom UI with function icons and
> locations like the stack frame sidebar section. To unify how we show stack
> traces.

Ooo, that is a great idea!

[Joseph Pecoraro]

http://trac.webkit.org/changeset/182644

[Joseph Pecoraro]

And I made an error in my post-review changes that got fixed with:
http://trac.webkit.org/changeset/182646