Bug 149708 – Web Inspector: Clip string previews

• Component: Web Inspector
• Status: RESOLVED FIXED

[Nikita Vasilyev]

Created attachment 262256 [details]
[Image] Bug

Currently, console.log("A 2MB string", aLongLongString) shows the whole string in the preview. There is no limit on how long a string can be.

Hovering over aLongLongString in the debugger will show the whole string on a single line.

We should clip long strings in the previews.

[Radar WebKit Bug Importer]

<rdar://problem/22933240>

[Nikita Vasilyev]

Created attachment 262257 [details]
[HTML] Reduction

[Nikita Vasilyev]

Created attachment 262259 [details]
Patch

[Nikita Vasilyev]

Created attachment 262262 [details]
[Animated GIF] With the patch applied

[Nikita Vasilyev]

Created attachment 262263 [details]
[Animated GIF] With the patch applied (stack traces)

The patch makes console messages with clipped strings expandable, so it also mitigates "Web Inspector: Can't click on a stack trace link when it's in a console message preview" https://bugs.webkit.org/show_bug.cgi?id=149638.

[Devin Rousso]

This makes sense to me, but I think that the clipping could be a bit smarter.  Is there a reason that you didn't have it attempt to find a \s somewhere immediately before the 140 character limit to provide "nicer" splitting?  If there is a stream of characters with no breaks (like a URL) then this makes sense, but for words I think it should try splitting more nicely.  Otherwise, super awesome.

[Nikita Vasilyev]

Created attachment 262268 [details]
Patch

[Nikita Vasilyev]

Comment on attachment 262268 [details]
Patch

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

> Source/WebInspectorUI/UserInterface/Base/Utilities.js:481
> +        "use strict";

If I don't use strict, "abc".truncate(3) would create a new object: https://cloudup.com/cRM82I1lE8d
Using strict makes it no-op.

> Source/WebInspectorUI/UserInterface/Base/Utilities.js:488
> +        if (matched && matched[0].length < Math.floor(maxLength / 2))

"1 234".truncate(5) should return "1 23…" and not "1…", so I used this Math.floor(maxLength / 2)

[Joseph Pecoraro]

Comment on attachment 262268 [details]
Patch

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

Looks fine to me. But some questions. r=me

>> Source/WebInspectorUI/UserInterface/Base/Utilities.js:481
>> +        "use strict";
> 
> If I don't use strict, "abc".truncate(3) would create a new object: https://cloudup.com/cRM82I1lE8d
> Using strict makes it no-op.

Wow. Why is that?

> Source/WebInspectorUI/UserInterface/Base/Utilities.js:487
> +        let matched = clipped.match(/\s[^\s]*$/);

I think it would be clearer to do something like:

let indexOfLastWhitespace = clipped.search(/\s\S*$/);
if (indexOfLastWhitespace > Math.floor(maxLength / 2))
    clipped = clipped.slice(0, indexOfLastWhitespace - 1);

For a while I didn't understand what you were doing here.

[Nikita Vasilyev]

Comment on attachment 262268 [details]
Patch

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

>>> Source/WebInspectorUI/UserInterface/Base/Utilities.js:481
>>> +        "use strict";
>> 
>> If I don't use strict, "abc".truncate(3) would create a new object: https://cloudup.com/cRM82I1lE8d
>> Using strict makes it no-op.
> 
> Wow. Why is that?

In non-strict mode "this" is always an object. Prototype methods on primitives (numbers, strings, boolean) force to create an object on every call. Strict mode allows "this" to be a primitive.

>> Source/WebInspectorUI/UserInterface/Base/Utilities.js:487
>> +        let matched = clipped.match(/\s[^\s]*$/);
> 
> I think it would be clearer to do something like:
> 
> let indexOfLastWhitespace = clipped.search(/\s\S*$/);
> if (indexOfLastWhitespace > Math.floor(maxLength / 2))
>     clipped = clipped.slice(0, indexOfLastWhitespace - 1);
> 
> For a while I didn't understand what you were doing here.

Completely forgot about the search method, thanks!

[Nikita Vasilyev]

Created attachment 262272 [details]
Patch

[WebKit Commit Bot]

Comment on attachment 262272 [details]
Patch

Clearing flags on attachment: 262272

Committed r190426: <http://trac.webkit.org/changeset/190426>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.