- Revision
- 233817
- Author
- [email protected]
- Date
- 2018-07-13 14:53:14 -0700 (Fri, 13 Jul 2018)
Log Message
Web Inspector: Execution highlighting in the frontend should be line/column-based
https://bugs.webkit.org/show_bug.cgi?id=187532
<rdar://problem/42035580>
Reviewed by Joseph Pecoraro.
Source code offsets from Esprima should not be used to calculate ranges
in CodeMirror for _expression_ highlighting.
This also fixes a long standing bug when adjusting for the starting
position of an inline script. Previously the start offset from the script
TextRange was used for this purpose, but the value is often incorrect (see
https://bugs.webkit.org/show_bug.cgi?id=187532#c5). By using the starting
line/column instead, we avoid the problem.
* UserInterface/Models/ScriptSyntaxTree.js:
(WI.ScriptSyntaxTree.prototype.containersOfPosition):
(WI.ScriptSyntaxTree.prototype.containersOfOffset): Deleted.
* UserInterface/Models/SourceCodePosition.js:
(WI.SourceCodePosition.prototype.offsetColumn):
* UserInterface/Views/SourceCodeTextEditor.js:
(WI.SourceCodeTextEditor.prototype.textEditorExecutionHighlightRange.toInlineScriptPosition):
(WI.SourceCodeTextEditor.prototype.textEditorExecutionHighlightRange.fromInlineScriptPosition):
(WI.SourceCodeTextEditor.prototype.textEditorExecutionHighlightRange):
(WI.SourceCodeTextEditor.prototype.textEditorExecutionHighlightRange.convertRangeOffsetsToSourceCodeOffsets): Deleted.
* UserInterface/Views/TextEditor.js:
(WI.TextEditor.prototype._updateExecutionRangeHighlight):
Modified Paths
Diff
Modified: trunk/Source/WebInspectorUI/ChangeLog (233816 => 233817)
--- trunk/Source/WebInspectorUI/ChangeLog 2018-07-13 20:56:36 UTC (rev 233816)
+++ trunk/Source/WebInspectorUI/ChangeLog 2018-07-13 21:53:14 UTC (rev 233817)
@@ -1,5 +1,38 @@
2018-07-13 Matt Baker <[email protected]>
+ Web Inspector: Execution highlighting in the frontend should be line/column-based
+ https://bugs.webkit.org/show_bug.cgi?id=187532
+ <rdar://problem/42035580>
+
+ Reviewed by Joseph Pecoraro.
+
+ Source code offsets from Esprima should not be used to calculate ranges
+ in CodeMirror for _expression_ highlighting.
+
+ This also fixes a long standing bug when adjusting for the starting
+ position of an inline script. Previously the start offset from the script
+ TextRange was used for this purpose, but the value is often incorrect (see
+ https://bugs.webkit.org/show_bug.cgi?id=187532#c5). By using the starting
+ line/column instead, we avoid the problem.
+
+ * UserInterface/Models/ScriptSyntaxTree.js:
+ (WI.ScriptSyntaxTree.prototype.containersOfPosition):
+ (WI.ScriptSyntaxTree.prototype.containersOfOffset): Deleted.
+
+ * UserInterface/Models/SourceCodePosition.js:
+ (WI.SourceCodePosition.prototype.offsetColumn):
+
+ * UserInterface/Views/SourceCodeTextEditor.js:
+ (WI.SourceCodeTextEditor.prototype.textEditorExecutionHighlightRange.toInlineScriptPosition):
+ (WI.SourceCodeTextEditor.prototype.textEditorExecutionHighlightRange.fromInlineScriptPosition):
+ (WI.SourceCodeTextEditor.prototype.textEditorExecutionHighlightRange):
+ (WI.SourceCodeTextEditor.prototype.textEditorExecutionHighlightRange.convertRangeOffsetsToSourceCodeOffsets): Deleted.
+
+ * UserInterface/Views/TextEditor.js:
+ (WI.TextEditor.prototype._updateExecutionRangeHighlight):
+
+2018-07-13 Matt Baker <[email protected]>
+
Web Inspector: SourceCodePosition.js missing from Test.html
https://bugs.webkit.org/show_bug.cgi?id=187644
Modified: trunk/Source/WebInspectorUI/UserInterface/Models/ScriptSyntaxTree.js (233816 => 233817)
--- trunk/Source/WebInspectorUI/UserInterface/Models/ScriptSyntaxTree.js 2018-07-13 20:56:36 UTC (rev 233816)
+++ trunk/Source/WebInspectorUI/UserInterface/Models/ScriptSyntaxTree.js 2018-07-13 21:53:14 UTC (rev 233817)
@@ -1,5 +1,5 @@
/*
- * Copyright (C) 2014, 2015 Apple Inc. All rights reserved.
+ * Copyright (C) 2014-2018 Apple Inc. All rights reserved.
*
* Redistribution and use in source and binary forms, with or without
* modification, are permitted provided that the following conditions
@@ -79,7 +79,7 @@
return nodes;
}
- containersOfOffset(offset)
+ containersOfPosition(position)
{
console.assert(this._parsedSuccessfully);
if (!this._parsedSuccessfully)
@@ -86,15 +86,13 @@
return [];
let allNodes = [];
- const start = 0;
- const end = 1;
this.forEachNode((node, state) => {
- if (node.range[end] < offset)
+ if (node.endPosition.isBefore(position))
state.skipChildNodes = true;
- if (node.range[start] > offset)
+ else if (node.startPosition.isAfter(position))
state.shouldStopEarly = true;
- if (node.range[start] <= offset && node.range[end] >= offset)
+ else
allNodes.push(node);
});
Modified: trunk/Source/WebInspectorUI/UserInterface/Models/SourceCodePosition.js (233816 => 233817)
--- trunk/Source/WebInspectorUI/UserInterface/Models/SourceCodePosition.js 2018-07-13 20:56:36 UTC (rev 233816)
+++ trunk/Source/WebInspectorUI/UserInterface/Models/SourceCodePosition.js 2018-07-13 21:53:14 UTC (rev 233817)
@@ -1,5 +1,5 @@
/*
- * Copyright (C) 2013 Apple Inc. All rights reserved.
+ * Copyright (C) 2013-2018 Apple Inc. All rights reserved.
*
* Redistribution and use in source and binary forms, with or without
* modification, are permitted provided that the following conditions
@@ -36,6 +36,12 @@
get lineNumber() { return this._lineNumber; }
get columnNumber() { return this._columnNumber; }
+ offsetColumn(delta)
+ {
+ console.assert(this._columnNumber + delta >= 0);
+ return new WI.SourceCodePosition(this._lineNumber, this._columnNumber + delta);
+ }
+
equals(position)
{
return this._lineNumber === position.lineNumber && this._columnNumber === position.columnNumber;
Modified: trunk/Source/WebInspectorUI/UserInterface/Views/SourceCodeTextEditor.js (233816 => 233817)
--- trunk/Source/WebInspectorUI/UserInterface/Views/SourceCodeTextEditor.js 2018-07-13 20:56:36 UTC (rev 233816)
+++ trunk/Source/WebInspectorUI/UserInterface/Views/SourceCodeTextEditor.js 2018-07-13 21:53:14 UTC (rev 233817)
@@ -1,5 +1,5 @@
/*
- * Copyright (C) 2013, 2015 Apple Inc. All rights reserved.
+ * Copyright (C) 2013-2018 Apple Inc. All rights reserved.
*
* Redistribution and use in source and binary forms, with or without
* modification, are permitted provided that the following conditions
@@ -1411,8 +1411,11 @@
this._reinsertAllThreadIndicators();
}
- textEditorExecutionHighlightRange(offset, position, characterAtOffset, callback)
+ textEditorExecutionHighlightRange(currentPosition, callback)
{
+ let {line, ch} = this.currentPositionToOriginalPosition(currentPosition);
+ let position = new WI.SourceCodePosition(line, ch);
+
let script = this._getAssociatedScript(position);
if (!script) {
callback(null);
@@ -1419,17 +1422,29 @@
return;
}
- // If this is an inline script, then convert to offset within the inline script.
- let adjustment = script.range.startOffset || 0;
- offset = offset - adjustment;
+ let {startLine, startColumn} = script.range;
+ function toInlineScriptPosition(position) {
+ let columnNumber = position.lineNumber === startLine ? position.columnNumber - startColumn : position.columnNumber;
+ return new WI.SourceCodePosition(position.lineNumber - startLine, columnNumber);
+ }
+
+ function fromInlineScriptPosition(position) {
+ let columnNumber = position.lineNumber ? position.columnNumber : position.columnNumber + startColumn;
+ return new WI.SourceCodePosition(position.lineNumber + startLine, columnNumber);
+ }
+
// When returning offsets, convert to offsets within the SourceCode being viewed.
- function convertRangeOffsetsToSourceCodeOffsets(range) {
- return range.map((offset) => offset + adjustment);
+ let highlightSourceCodeRange = (startPosition, endPosition) => {
+ startPosition = fromInlineScriptPosition(startPosition).toCodeMirror();
+ endPosition = fromInlineScriptPosition(endPosition).toCodeMirror();
+ callback({startPosition, endPosition});
}
script.requestScriptSyntaxTree((syntaxTree) => {
- let nodes = syntaxTree.containersOfOffset(offset);
+ // Convert to the position within the inline script before querying the AST.
+ position = toInlineScriptPosition(position);
+ let nodes = syntaxTree.containersOfPosition(position);
if (!nodes.length) {
callback(null);
return;
@@ -1439,18 +1454,17 @@
// Avoid highlighting the entire program if this is the start of the first statement.
// Special case the assignment _expression_ inside of a for..of and for..in to highlight a larger range.
for (let node of nodes) {
- let startOffset = node.range[0];
- if (startOffset === offset && node.type !== WI.ScriptSyntaxTree.NodeType.Program) {
- callback(convertRangeOffsetsToSourceCodeOffsets(node.range));
+ if (node.startPosition.equals(position) && node.type !== WI.ScriptSyntaxTree.NodeType.Program) {
+ highlightSourceCodeRange(node.startPosition, node.endPosition);
return;
}
if (node.type === WI.ScriptSyntaxTree.NodeType.ForInStatement || node.type === WI.ScriptSyntaxTree.NodeType.ForOfStatement) {
- if (node.left.range[0] === offset) {
- callback(convertRangeOffsetsToSourceCodeOffsets([node.left.range[0], node.right.range[1]]));
+ if (node.left.startPosition.equals(position)) {
+ highlightSourceCodeRange(node.left.startPosition, node.right.endPosition);
return;
}
}
- if (startOffset > offset)
+ if (node.startPosition.isAfter(position))
break;
}
@@ -1458,16 +1472,14 @@
// We check this after ensuring nothing starts with this offset,
// as that would be more important.
for (let node of nodes) {
- let startOffset = node.range[0];
- let endOffset = node.range[1];
- if (endOffset === offset) {
+ if (node.endPosition.equals(position)) {
if (node.type === WI.ScriptSyntaxTree.NodeType.BlockStatement) {
// Closing brace of a block, only highlight the closing brace character.
- callback(convertRangeOffsetsToSourceCodeOffsets([offset - 1, offset]));
+ highlightSourceCodeRange(position.offsetColumn(-1), position);
return;
}
}
- if (startOffset > offset)
+ if (node.startPosition.isAfter(position))
break;
}
@@ -1479,7 +1491,8 @@
return aLength - bLength;
});
- let characterAtOffsetIsDotOrBracket = characterAtOffset === "." || characterAtOffset === "[";
+ let characterAtPosition = this.getTextInRange(currentPosition, {line: currentPosition.line, ch: currentPosition.ch + 1});
+ let characterAtPositionIsDotOrBracket = characterAtPosition === "." || characterAtPosition === "[";
for (let i = 0; i < nodes.length; ++i) {
let node = nodes[i];
@@ -1488,7 +1501,7 @@
if (node.type === WI.ScriptSyntaxTree.NodeType.CallExpression
|| node.type === WI.ScriptSyntaxTree.NodeType.NewExpression
|| node.type === WI.ScriptSyntaxTree.NodeType.ThrowStatement) {
- callback(convertRangeOffsetsToSourceCodeOffsets(node.range));
+ highlightSourceCodeRange(node.startPosition, node.endPosition);
return;
}
@@ -1507,13 +1520,13 @@
// foo["x"]*["y"]["z"] => inside y looking at parent call frame => |foo["x"]["y"]|["z"]
//
if (node.type === WI.ScriptSyntaxTree.NodeType.ThisExpression
- || (characterAtOffsetIsDotOrBracket && (node.type === WI.ScriptSyntaxTree.NodeType.Identifier || node.type === WI.ScriptSyntaxTree.NodeType.MemberExpression))) {
+ || (characterAtPositionIsDotOrBracket && (node.type === WI.ScriptSyntaxTree.NodeType.Identifier || node.type === WI.ScriptSyntaxTree.NodeType.MemberExpression))) {
let memberExpressionNode = null;
for (let j = i + 1; j < nodes.length; ++j) {
let nextNode = nodes[j];
if (nextNode.type === WI.ScriptSyntaxTree.NodeType.MemberExpression) {
memberExpressionNode = nextNode;
- if (offset === memberExpressionNode.range[1])
+ if (position.equals(memberExpressionNode.endPosition))
continue;
}
break;
@@ -1520,11 +1533,11 @@
}
if (memberExpressionNode) {
- callback(convertRangeOffsetsToSourceCodeOffsets(memberExpressionNode.range));
+ highlightSourceCodeRange(memberExpressionNode.startPosition, memberExpressionNode.endPosition);
return;
}
- callback(convertRangeOffsetsToSourceCodeOffsets(node.range));
+ highlightSourceCodeRange(node.startPosition, node.endPosition);
return;
}
}
Modified: trunk/Source/WebInspectorUI/UserInterface/Views/TextEditor.js (233816 => 233817)
--- trunk/Source/WebInspectorUI/UserInterface/Views/TextEditor.js 2018-07-13 20:56:36 UTC (rev 233816)
+++ trunk/Source/WebInspectorUI/UserInterface/Views/TextEditor.js 2018-07-13 21:53:14 UTC (rev 233817)
@@ -1324,12 +1324,8 @@
return;
let currentPosition = {line: this._executionLineNumber, ch: this._executionColumnNumber};
- let originalOffset = this.currentPositionToOriginalOffset(currentPosition);
- let originalCodeMirrorPosition = this.currentPositionToOriginalPosition(currentPosition);
- let originalPosition = new WI.SourceCodePosition(originalCodeMirrorPosition.line, originalCodeMirrorPosition.ch);
- let characterAtOffset = this._codeMirror.getRange(currentPosition, {line: this._executionLineNumber, ch: this._executionColumnNumber + 1});
- this._delegate.textEditorExecutionHighlightRange(originalOffset, originalPosition, characterAtOffset, (range) => {
+ this._delegate.textEditorExecutionHighlightRange(currentPosition, (range) => {
let start, end;
if (!range) {
// Highlight the rest of the line.
@@ -1337,8 +1333,8 @@
end = {line: this._executionLineNumber};
} else {
// Highlight the range.
- start = this.originalOffsetToCurrentPosition(range[0]);
- end = this.originalOffsetToCurrentPosition(range[1]);
+ start = range.startPosition;
+ end = range.endPosition;
}
// Ensure the marker is cleared in case there were multiple updates very quickly.
