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.
<rdar://problem/22933240>
Created attachment 262257 [details] [HTML] Reduction
Created attachment 262259 [details] Patch
Created attachment 262262 [details] [Animated GIF] With the patch applied
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.
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.
Created attachment 262268 [details] Patch
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)
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.
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!
Created attachment 262272 [details] Patch
Comment on attachment 262272 [details] Patch Clearing flags on attachment: 262272 Committed r190426: <http://trac.webkit.org/changeset/190426>
All reviewed patches have been landed. Closing bug.