Bug 160465 – Web Inspector: CodeMirror in resource content view should not show 'CR' characters

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

[Blaze Burg]

SUMMARY:

When pretty-printing is disabled, or for comment blocks (which aren't altered by pretty-printing), some pages show a 'CR' glyph at the end of each line. This is probably a CR/LF character. We shouldn't render this glyph as it's distracting and irrelevant for JavaScript.

STEPS TO REPRODUCE:

1. Go to the URL
2. Go to resources tab
3. Select 'foresee_surveydef.js'  or any other JS file served from 'gateway.answerscloud.com'
4. Deselect pretty printing (the {} button)

Now most of the lines have a little 'CR' glyph at the end. This is annoying.

[Radar WebKit Bug Importer]

<rdar://problem/27666053>

[Nikita Vasilyev]

https://github.com/WebKit/webkit/blob/c20f470adf2cf34821a0756d01a4df33b353c1e6/Source/WebInspectorUI/UserInterface/External/CodeMirror/codemirror.js#L6951

This appears to be hardcoded in CodeMirror, i.e. CodeMirror doesn't seem to
provide an option to turn it off. We may have to modify codemirror.js directly.

[Blaze Burg]

I don't think editing codemirror.js is going to be the right fix. Here are some alternatives:

1. Heuristically detect whether the line endings seem to be \r\n or \n and tell CodeMirror which line ending to assume (there's a setting for that)
2. Strip line endings from the text that we display (somewhat dangerous due to line:col interactions with JSC)
3. Tag the CR character with CSS and make it visibility:none.

[Nikita Vasilyev]

(In reply to comment #3)
> I don't think editing codemirror.js is going to be the right fix. Here are
> some alternatives:
> 
> 1. Heuristically detect whether the line endings seem to be \r\n or \n and
> tell CodeMirror which line ending to assume (there's a setting for that)

This is what Chrome seems to do.
https://codereview.chromium.org/19540026/

> 2. Strip line endings from the text that we display (somewhat dangerous due
> to line:col interactions with JSC)
> 3. Tag the CR character with CSS and make it visibility:none.

I can't figure out how to write a valid CSS selector for this element:

<span class="cm-invalidchar" cm-text="
">␍</span>

These don't work:

.cm-invalidchar[cm-text="
"]
.cm-invalidchar[cm-text="\n"]
.cm-invalidchar[cm-text="\a"]

[Timothy Hatcher]

We could just hide all .cm-invalidchar. This should be a setting switch once we have a Settings view.

[Devin Rousso]

Created attachment 287986 [details]
Patch

[Blaze Burg]

Comment on attachment 287986 [details]
Patch

How does this look on misencoded files? Is there any way to differentiate CR from other invalid characters?

[Devin Rousso]

(In reply to comment #7)
> How does this look on misencoded files? Is there any way to differentiate CR
> from other invalid characters?

Not really sure if I tested that.  Any sure-fire steps for reproducing a mis-encoded file?  What would that even look like?

[Blaze Burg]

Comment on attachment 287986 [details]
Patch

r=me

[Blaze Burg]

(In reply to comment #8)
> (In reply to comment #7)
> > How does this look on misencoded files? Is there any way to differentiate CR
> > from other invalid characters?
> 
> Not really sure if I tested that.  Any sure-fire steps for reproducing a
> mis-encoded file?  What would that even look like?

On second thought, I've only really seen this on XHR responses that had the wrong MIME type. I think Johan fixed a bug in that area. Not sure this would really make it any worse...

[WebKit Commit Bot]

Comment on attachment 287986 [details]
Patch

Clearing flags on attachment: 287986

Committed r205517: <http://trac.webkit.org/changeset/205517>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.