Bug 187532

Summary: Web Inspector: Execution highlighting in the frontend should be line/column-based
Product: WebKit Reporter: Matt Baker <mattbaker>
Component: Web InspectorAssignee: Matt Baker <mattbaker>
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue, inspector-bugzilla-changes, joepeck, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: All   
OS: All   
Bug Depends on: 187612    
Bug Blocks: 186453    
Attachments:
Description Flags
Patch
none
Patch
none
Patch none

Matt Baker
Reported 2018-07-10 12:36:20 PDT
Summary: Execution highlighting in the frontend should be line/column-based. The call frame location payload uses line/column, but to simplify range calculations in the frontend, TextEditor converts these to character offsets when highlighting source code locations. The frontend should never use raw offsets into source code, since the values will depend on the particular line endings used. This patch is a step toward allowing CodeMirror to auto-detect line endings again, instead of forcing Unix-style (\n) line endings. Specifying a line ending other than that of the original file causes problems when inspecting certain resources (see https://webkit.org/b/186453).
Attachments
Patch (18.99 KB, patch)
2018-07-10 13:00 PDT, Matt Baker 		no flags
DetailsFormatted DiffDiff
Patch (14.65 KB, patch)
2018-07-13 08:47 PDT, Matt Baker 		no flags
DetailsFormatted DiffDiff
Patch (14.98 KB, patch)
2018-07-13 09:18 PDT, Matt Baker 		no flags
DetailsFormatted DiffDiff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.
Radar WebKit Bug Importer
Comment 1 2018-07-10 12:36:46 PDT
<rdar://problem/42035580>
Matt Baker
Comment 2 2018-07-10 13:00:09 PDT
Created attachment 344716 [details] Patch
Joseph Pecoraro
Comment 3 2018-07-10 15:04:17 PDT
Again I have a very bad feeling about this. Changing line endings affects many things, offsets was supposed to be an easy solution avoiding line endings. Is this fixing a recent regression? If so, what caused it?
Matt Baker
Comment 4 2018-07-12 12:26:02 PDT
I think we should land https://webkit.org/b/187612 first. It is a simpler change, and duplicates some of the changes here, allowing this patch to be smaller.
Matt Baker
Comment 5 2018-07-12 22:06:48 PDT
It looks like expression highlighting in inline scripts is currently broken in Safari, even in files with Unix-style line endings. For example, // test.js var o = { get x() { debugger; return 1; } }; var x = o.x; // test.html <html> <head> <script> var o = { get x() { debugger; return 1; } }; var x = o.x; </script> <script src="test.js"></script> </head> </html> Steps to Reproduce: 1. Inspect the test page 2. Reload the page 3. Debugger breaks at test.html:6 4. Select the call frame's parent Expected: > var x = |o.x|; Actual: > var x |= o|.x; When breaking at the second debugger statement (in the separate script file) the parent call frame's selection appears correctly.
Matt Baker
Comment 6 2018-07-13 08:47:57 PDT
Created attachment 344948 [details] Patch
Matt Baker
Comment 7 2018-07-13 09:18:29 PDT
Created attachment 344950 [details] Patch
Matt Baker
Comment 8 2018-07-13 09:19:14 PDT
(In reply to Matt Baker from comment #7) > Created attachment 344950 [details] > Patch This also addresses the long standing bug with expression highlighting in inline scripts: https://bugs.webkit.org/show_bug.cgi?id=187532#c5.
Joseph Pecoraro
Comment 9 2018-07-13 12:24:22 PDT
Comment on attachment 344950 [details] Patch r=me Nice! I think using SourcePosition everywhere is cleaning things up a bit!
WebKit Commit Bot
Comment 10 2018-07-13 14:53:21 PDT
Comment on attachment 344950 [details] Patch Clearing flags on attachment: 344950 Committed r233817: <https://trac.webkit.org/changeset/233817>
WebKit Commit Bot
Comment 11 2018-07-13 14:53:23 PDT
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.