Bug 153529

Summary: CodeMirror will strip out "\r" from files with "\r\n" as newlines causing our offsets into the file to be incorrect
Product: WebKit Reporter: Saam Barati <saam>
Component: Web InspectorAssignee: Saam Barati <saam>
Status: RESOLVED FIXED    
Severity: Normal CC: bburg, benjamin, burg, commit-queue, fpizlo, ggaren, graouts, gskachkov, joepeck, keith_miller, mark.lam, mattbaker, msaboff, nvasilyev, oliver, sukolsak, timothy, webkit-bug-importer, ysuzuki
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
example file
none
image
none
patch none

Saam Barati
Reported 2016-01-26 19:08:11 PST
I'm guessing this is parser related.
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
DetailsFormatted DiffDiff
View All Add attachment proposed patch, testcase, etc.
Saam Barati
Comment 1 2016-01-26 19:08:55 PST
Created attachment 269972 [details] example file
Saam Barati
Comment 2 2016-01-26 19:10:02 PST
Created attachment 269973 [details] image what that file looks like in inspector
Saam Barati
Comment 3 2016-01-27 14:26:14 PST
This is a bug relating to how we use CodeMirror.
Radar WebKit Bug Importer
Comment 4 2016-01-27 14:26:41 PST
<rdar://problem/24376799>
Radar WebKit Bug Importer
Comment 5 2016-01-27 14:28:17 PST
<rdar://problem/24376840>
Saam Barati
Comment 6 2016-01-27 14:47:52 PST
Created attachment 270047 [details] patch
Blaze Burg
Comment 7 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?
Saam Barati
Comment 8 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"
Timothy Hatcher
Comment 9 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.
Timothy Hatcher
Comment 10 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;
Saam Barati
Comment 11 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.
WebKit Commit Bot
Comment 12 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>
WebKit Commit Bot
Comment 13 2016-01-27 18:14:46 PST
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.