Bug 153529 - CodeMirror will strip out "\r" from files with "\r\n" as newlines causing our offsets into the file to be incorrect
Summary: CodeMirror will strip out "\r" from files with "\r\n" as newlines causing our...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Inspector (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Saam Barati
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2016-01-26 19:08 PST by Saam Barati
Modified: 2016-01-27 18:14 PST (History)
19 users (show)

See Also:

Attachments
example file (301 bytes, application/x-javascript)
2016-01-26 19:08 PST, Saam Barati 		no flags Details
image (52.61 KB, image/png)
2016-01-26 19:10 PST, Saam Barati 		no flags Details
patch (2.72 KB, patch)
2016-01-27 14:47 PST, Saam Barati 		no flags Details | Formatted Diff | Diff
View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Saam Barati 2016-01-26 19:08:11 PST 
I'm guessing this is parser related.
Comment 1 Saam Barati 2016-01-26 19:08:55 PST 
Created attachment 269972 [details]
example file
Comment 2 Saam Barati 2016-01-26 19:10:02 PST 
Created attachment 269973 [details]
image

what that file looks like in inspector
Comment 3 Saam Barati 2016-01-27 14:26:14 PST 
This is a bug relating to how we use CodeMirror.
Comment 4 Radar WebKit Bug Importer 2016-01-27 14:26:41 PST 
<rdar://problem/24376799>
Comment 5 Radar WebKit Bug Importer 2016-01-27 14:28:17 PST 
<rdar://problem/24376840>
Comment 6 Saam Barati 2016-01-27 14:47:52 PST 
Created attachment 270047 [details]
patch
Comment 7 BJ Burg 2016-01-27 14:51:10 PST 
Comment on attachment 270047 [details]
patch

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

> Source/WebInspectorUI/UserInterface/Views/CodeMirrorEditor.js:31
> +            options.lineSeparator = "\n";

Why does this make CodeMirror leave the \r alone?
Comment 8 Saam Barati 2016-01-27 15:20:37 PST 
(In reply to comment #7)
> Comment on attachment 270047 [details]
> patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=270047&action=review
> 
> > Source/WebInspectorUI/UserInterface/Views/CodeMirrorEditor.js:31
> > +            options.lineSeparator = "\n";
> 
> Why does this make CodeMirror leave the \r alone?

It will trust us and use "\n" as the newline terminator
instead of its own default thing that takes into account
"\r"
Comment 9 Timothy Hatcher 2016-01-27 15:21:47 PST 
Comment on attachment 270047 [details]
patch

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

>> Source/WebInspectorUI/UserInterface/Views/CodeMirrorEditor.js:31
>> +            options.lineSeparator = "\n";
> 
> Why does this make CodeMirror leave the \r alone?

I suspect the \r will be there in the editor on the end of the lines, but will be invisible.
Comment 10 Timothy Hatcher 2016-01-27 15:23:29 PST 
Does CodeMirror render the \r? From the code it looks like it might:

 var txt = content.appendChild(elt("span", m[0] == "\r" ? "␍" : "␤", "cm-invalidchar"));
 txt.setAttribute("cm-text", m[0]);
 builder.col += 1;
Comment 11 Saam Barati 2016-01-27 17:20:52 PST 
(In reply to comment #10)
> Does CodeMirror render the \r? From the code it looks like it might:
> 
>  var txt = content.appendChild(elt("span", m[0] == "\r" ? "␍" : "␤",
> "cm-invalidchar"));
>  txt.setAttribute("cm-text", m[0]);
>  builder.col += 1;

It does.
Comment 12 WebKit Commit Bot 2016-01-27 18:14:41 PST 
Comment on attachment 270047 [details]
patch

Clearing flags on attachment: 270047

Committed r195723: <http://trac.webkit.org/changeset/195723>
Comment 13 WebKit Commit Bot 2016-01-27 18:14:46 PST 
All reviewed patches have been landed.  Closing bug.