Bug 187532 - Web Inspector: Execution highlighting in the frontend should be line/column-based
Summary: Web Inspector: Execution highlighting in the frontend should be line/column-b...
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: Matt Baker
URL:
Keywords: InRadar
Depends on: 187612
Blocks: 186453
  Show dependency treegraph
 
Reported: 2018-07-10 12:36 PDT by Matt Baker
Modified: 2018-07-13 14:53 PDT (History)
4 users (show)

See Also:


Attachments
Patch (18.99 KB, patch)
2018-07-10 13:00 PDT, Matt Baker
no flags Details | Formatted Diff | Diff
Patch (14.65 KB, patch)
2018-07-13 08:47 PDT, Matt Baker
no flags Details | Formatted Diff | Diff
Patch (14.98 KB, patch)
2018-07-13 09:18 PDT, Matt Baker
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Matt Baker 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).
Comment 1 Radar WebKit Bug Importer 2018-07-10 12:36:46 PDT
<rdar://problem/42035580>
Comment 2 Matt Baker 2018-07-10 13:00:09 PDT
Created attachment 344716 [details]
Patch
Comment 3 Joseph Pecoraro 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?
Comment 4 Matt Baker 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.
Comment 5 Matt Baker 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.
Comment 6 Matt Baker 2018-07-13 08:47:57 PDT
Created attachment 344948 [details]
Patch
Comment 7 Matt Baker 2018-07-13 09:18:29 PDT
Created attachment 344950 [details]
Patch
Comment 8 Matt Baker 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.
Comment 9 Joseph Pecoraro 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!
Comment 10 WebKit Commit Bot 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>
Comment 11 WebKit Commit Bot 2018-07-13 14:53:23 PDT
All reviewed patches have been landed.  Closing bug.