Bug 190528 - Web Inspector: sequences of spaces longer than 16 don't show a dot
Summary: Web Inspector: sequences of spaces longer than 16 don't show a dot
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Inspector (show other bugs)
Version: WebKit Nightly Build
Hardware: All All
: P2 Normal
Assignee: Devin Rousso
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2018-10-12 10:55 PDT by Devin Rousso
Modified: 2018-10-16 09:48 PDT (History)
5 users (show)

See Also:


Attachments
[Image] Screenshot of Issue (85.41 KB, image/png)
2018-10-12 10:55 PDT, Devin Rousso
no flags Details
Patch (6.79 KB, patch)
2018-10-12 17:26 PDT, Devin Rousso
no flags Details | Formatted Diff | Diff
Patch (6.86 KB, patch)
2018-10-16 09:25 PDT, Devin Rousso
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Devin Rousso 2018-10-12 10:55:33 PDT
Created attachment 352181 [details]
[Image] Screenshot of Issue

We should probably programmatically generate the CSS space rules instead of manually writing 16 of them.
Comment 1 Devin Rousso 2018-10-12 17:26:35 PDT
Created attachment 352245 [details]
Patch
Comment 2 Matt Baker 2018-10-15 09:59:42 PDT
Comment on attachment 352245 [details]
Patch

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

r=me, nice cleanup!

> Source/WebInspectorUI/UserInterface/Views/CodeMirrorAdditions.js:369
> +                        styleText += `content: "${"\\00B7".repeat(count)}";`;

Nit: I think '\\00B7' would make this a little easier to read, since the "outer" double-quotes are part of the string, and the "inner" double-quotes are part of the expression.
Comment 3 Joseph Pecoraro 2018-10-15 13:40:56 PDT
Comment on attachment 352245 [details]
Patch

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

> Source/WebInspectorUI/UserInterface/Views/CodeMirrorAdditions.js:372
> +                        whitespaceStyleElement.textContent = styleText;

Hmm, this looks like the browser would have to reparse the entire stylesheet every time a new value is added, which seems wasteful. Maybe just creating a new element might be more efficient. Or maybe this is just negligible.
Comment 4 Devin Rousso 2018-10-16 09:19:47 PDT
Comment on attachment 352245 [details]
Patch

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

>> Source/WebInspectorUI/UserInterface/Views/CodeMirrorAdditions.js:372
>> +                        whitespaceStyleElement.textContent = styleText;
> 
> Hmm, this looks like the browser would have to reparse the entire stylesheet every time a new value is added, which seems wasteful. Maybe just creating a new element might be more efficient. Or maybe this is just negligible.

This only happens each time we encounter a new number of sequential spaces.  In my experience, I haven't seen more than 32 in a row before.  I don't think this is something we need to worry about.
Comment 5 Devin Rousso 2018-10-16 09:25:06 PDT
Created attachment 352460 [details]
Patch
Comment 6 WebKit Commit Bot 2018-10-16 09:47:15 PDT
Comment on attachment 352460 [details]
Patch

Clearing flags on attachment: 352460

Committed r237186: <https://trac.webkit.org/changeset/237186>
Comment 7 WebKit Commit Bot 2018-10-16 09:47:16 PDT
All reviewed patches have been landed.  Closing bug.
Comment 8 Radar WebKit Bug Importer 2018-10-16 09:48:30 PDT
<rdar://problem/45308758>