https://bugs.webkit.org/show_bug.cgi?id=150009
<rdar://problem/
23062199>
Reviewed by Timothy Hatcher.
This addresses a few long standing issues:
- IssueMessage and ConsoleMessage should not fight eachother
- Displayed issue messages now correctly have format string formatting, e.g. console.error("Foo %s", str)
- IssueMessage wraps a ConsoleMessage, so we don't duplicate everything
- Gives ConsoleMessage a sourceCodeLocation (lazy)
- Since a sourceCodeLocation can have the exact SourceCode, if it was a Script
without a Resource, we can only show the error in the Script's editor.
* UserInterface/Models/CallFrame.js:
(WebInspector.CallFrame.fromPayload):
Prefer the script identifier lookup first. And from the Script go
to a resource if possible. This allows us to distinguish a location
that should be in a Script that doesn't have a Resource when there
exists a Resource with the same URL. This will soon be the case
for dyanamic <script> elements append to a document.
* UserInterface/Controllers/IssueManager.js:
(WebInspector.IssueManager.issueMatchSourceCode):
Consolidate all the different checks to this one function.
(WebInspector.IssueManager.prototype.issueWasAdded):
Create IssueMessages with ConsoleMessages.
(WebInspector.IssueManager.prototype.issuesForSourceCode):
Simplify now that we have the better check.
* UserInterface/Controllers/LogManager.js:
(WebInspector.LogManager.prototype.messageWasAdded):
Once a ConsoleMessage has been created (and modified `parameters` for us)
create the IssueMessage if it was an issue.
* UserInterface/Models/ConsoleMessage.js:
(WebInspector.ConsoleMessage.prototype.get sourceCodeLocation):
Lazily create a source code from the best possible location. This can
be the top call frame or the url/line/column combination.
* UserInterface/Models/IssueMessage.js:
(WebInspector.IssueMessage):
Creation and most properties just call through to a ConsoleMessage.
The `type` and `text` are Issue specific. Anything that uses location
data should use the sourceCodeLocation.
(WebInspector.IssueMessage.prototype.saveIdentityToCookie):
Fix implementation that didn't account for a null sourceCodeLocation.
(WebInspector.IssueMessage.prototype._formatTextIfNecessary):
Basic text format message formatting.
* UserInterface/Protocol/ConsoleObserver.js:
(WebInspector.ConsoleObserver.prototype.messageAdded):
No longer call IssueMessage from the observer. Let LogManager trigger issues.
* UserInterface/Views/ContentView.js:
(WebInspector.ContentView.createFromRepresentedObject):
(WebInspector.ContentView.resolvedRepresentedObjectForRepresentedObject):
(WebInspector.ContentView.isViewable):
An IssueMessage represented object for an IssueMessageTreeElement should be
restorable by just going to the sourceCodeLocation it references. This is
identical to a Breakpoint.
* UserInterface/Views/IssueTreeElement.js:
(WebInspector.IssueTreeElement.prototype._updateTitles):
(WebInspector.IssueTreeElement):
* UserInterface/Views/ResourceContentView.js:
(WebInspector.ResourceContentView.prototype._issueWasAdded):
* UserInterface/Views/SourceCodeTextEditor.js:
(WebInspector.SourceCodeTextEditor.prototype._issueWasAdded):
(WebInspector.SourceCodeTextEditor.prototype._addIssue):
(WebInspector.SourceCodeTextEditor.prototype._reinsertAllIssues):
(WebInspector.SourceCodeTextEditor.prototype._matchesIssue): Deleted.
Update to use Issue's sourceCodeLocation or IssueManager's new APIs.
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@200064
268f45cc-cd09-0410-ab3c-
d52691b4dbfc
+2016-04-25 Joseph Pecoraro <pecoraro@apple.com>
+
+ Web Inspector: Line error widget showed in the wrong resource
+ https://bugs.webkit.org/show_bug.cgi?id=150009
+ <rdar://problem/23062199>
+
+ Reviewed by Timothy Hatcher.
+
+ This addresses a few long standing issues:
+
+ - IssueMessage and ConsoleMessage should not fight eachother
+ - Displayed issue messages now correctly have format string formatting, e.g. console.error("Foo %s", str)
+ - IssueMessage wraps a ConsoleMessage, so we don't duplicate everything
+ - Gives ConsoleMessage a sourceCodeLocation (lazy)
+ - Since a sourceCodeLocation can have the exact SourceCode, if it was a Script
+ without a Resource, we can only show the error in the Script's editor.
+
+ * UserInterface/Models/CallFrame.js:
+ (WebInspector.CallFrame.fromPayload):
+ Prefer the script identifier lookup first. And from the Script go
+ to a resource if possible. This allows us to distinguish a location
+ that should be in a Script that doesn't have a Resource when there
+ exists a Resource with the same URL. This will soon be the case
+ for dyanamic <script> elements append to a document.
+
+ * UserInterface/Controllers/IssueManager.js:
+ (WebInspector.IssueManager.issueMatchSourceCode):
+ Consolidate all the different checks to this one function.
+
+ (WebInspector.IssueManager.prototype.issueWasAdded):
+ Create IssueMessages with ConsoleMessages.
+
+ (WebInspector.IssueManager.prototype.issuesForSourceCode):
+ Simplify now that we have the better check.
+
+ * UserInterface/Controllers/LogManager.js:
+ (WebInspector.LogManager.prototype.messageWasAdded):
+ Once a ConsoleMessage has been created (and modified `parameters` for us)
+ create the IssueMessage if it was an issue.
+
+ * UserInterface/Models/ConsoleMessage.js:
+ (WebInspector.ConsoleMessage.prototype.get sourceCodeLocation):
+ Lazily create a source code from the best possible location. This can
+ be the top call frame or the url/line/column combination.
+
+ * UserInterface/Models/IssueMessage.js:
+ (WebInspector.IssueMessage):
+ Creation and most properties just call through to a ConsoleMessage.
+ The `type` and `text` are Issue specific. Anything that uses location
+ data should use the sourceCodeLocation.
+
+ (WebInspector.IssueMessage.prototype.saveIdentityToCookie):
+ Fix implementation that didn't account for a null sourceCodeLocation.
+
+ (WebInspector.IssueMessage.prototype._formatTextIfNecessary):
+ Basic text format message formatting.
+
+ * UserInterface/Protocol/ConsoleObserver.js:
+ (WebInspector.ConsoleObserver.prototype.messageAdded):
+ No longer call IssueMessage from the observer. Let LogManager trigger issues.
+
+ * UserInterface/Views/ContentView.js:
+ (WebInspector.ContentView.createFromRepresentedObject):
+ (WebInspector.ContentView.resolvedRepresentedObjectForRepresentedObject):
+ (WebInspector.ContentView.isViewable):
+ An IssueMessage represented object for an IssueMessageTreeElement should be
+ restorable by just going to the sourceCodeLocation it references. This is
+ identical to a Breakpoint.
+
+ * UserInterface/Views/IssueTreeElement.js:
+ (WebInspector.IssueTreeElement.prototype._updateTitles):
+ (WebInspector.IssueTreeElement):
+ * UserInterface/Views/ResourceContentView.js:
+ (WebInspector.ResourceContentView.prototype._issueWasAdded):
+ * UserInterface/Views/SourceCodeTextEditor.js:
+ (WebInspector.SourceCodeTextEditor.prototype._issueWasAdded):
+ (WebInspector.SourceCodeTextEditor.prototype._addIssue):
+ (WebInspector.SourceCodeTextEditor.prototype._reinsertAllIssues):
+ (WebInspector.SourceCodeTextEditor.prototype._matchesIssue): Deleted.
+ Update to use Issue's sourceCodeLocation or IssueManager's new APIs.
+
2016-04-24 Matt Baker <mattbaker@apple.com>
Web Inspector: Can't sort by name/source code location columns in Timeline data grids
this.initialize();
}
+ static issueMatchSourceCode(issue, sourceCode)
+ {
+ if (sourceCode instanceof WebInspector.SourceMapResource)
+ return issue.sourceCodeLocation && issue.sourceCodeLocation.displaySourceCode === sourceCode;
+ if (sourceCode instanceof WebInspector.Resource)
+ return issue.url === sourceCode.url && (!issue.sourceCodeLocation || issue.sourceCodeLocation.sourceCode === sourceCode);
+ if (sourceCode instanceof WebInspector.Script)
+ return (issue.sourceCodeLocation && issue.sourceCodeLocation.sourceCode === sourceCode) || (!issue.sourceCodeLocation && issue.url === sourceCode.url);
+ return false;
+ }
+
// Public
initialize()
this.dispatchEventToListeners(WebInspector.IssueManager.Event.Cleared);
}
- issueWasAdded(source, level, text, url, lineNumber, columnNumber, parameters)
+ issueWasAdded(consoleMessage)
{
- var modifiedLineNumber;
- if (lineNumber) {
- console.assert(typeof lineNumber === "number");
- modifiedLineNumber = lineNumber - 1;
- }
+ let issue = new WebInspector.IssueMessage(consoleMessage);
- var issue = new WebInspector.IssueMessage(source, level, text, url, modifiedLineNumber, columnNumber, parameters);
this._issues.push(issue);
this.dispatchEventToListeners(WebInspector.IssueManager.Event.IssueWasAdded, {issue});
var issues = [];
for (var i = 0; i < this._issues.length; ++i) {
- // FIXME: Support issues based on Script identifiers too.
var issue = this._issues[i];
- if (issue.url === sourceCode.url)
+ if (WebInspector.IssueManager.issueMatchSourceCode(issue, sourceCode))
issues.push(issue);
}
if (parameters)
parameters = parameters.map(WebInspector.RemoteObject.fromPayload);
- var message = new WebInspector.ConsoleMessage(source, level, text, type, url, line, column, repeatCount, parameters, stackTrace, null);
+ let message = new WebInspector.ConsoleMessage(source, level, text, type, url, line, column, repeatCount, parameters, stackTrace, null);
+
this.dispatchEventToListeners(WebInspector.LogManager.Event.MessageAdded, {message});
+
+ if (message.level === "warning" || message.level === "error")
+ WebInspector.issueManager.issueWasAdded(message);
}
messagesCleared()
nativeCode = true;
url = null;
} else if (url || scriptId) {
- let sourceCode = WebInspector.frameResourceManager.resourceForURL(url);
+ let sourceCode = null;
+ if (scriptId) {
+ sourceCode = WebInspector.debuggerManager.scriptForIdentifier(scriptId);
+ if (sourceCode && sourceCode.resource)
+ sourceCode = sourceCode.resource;
+ }
+ if (!sourceCode)
+ sourceCode = WebInspector.frameResourceManager.resourceForURL(url);
if (!sourceCode)
sourceCode = WebInspector.debuggerManager.scriptsForURL(url)[0];
- if (!sourceCode && scriptId)
- sourceCode = WebInspector.debuggerManager.scriptForIdentifier(scriptId);
if (sourceCode) {
// The lineNumber is 1-based, but we expect 0-based.
console.assert(typeof source === "string");
console.assert(typeof level === "string");
console.assert(typeof message === "string");
- console.assert(!parameters || parameters.every(function(x) { return x instanceof WebInspector.RemoteObject; }));
+ console.assert(!parameters || parameters.every((x) => x instanceof WebInspector.RemoteObject));
this._source = source;
this._level = level;
this._messageText = message;
this._type = type || WebInspector.ConsoleMessage.MessageType.Log;
+
this._url = url || null;
this._line = line || 0;
this._column = column || 0;
+ this._sourceCodeLocation = undefined;
this._repeatCount = repeatCount || 0;
this._parameters = parameters;
// Public
- get source()
- {
- return this._source;
- }
-
- get level()
- {
- return this._level;
- }
-
- get messageText()
- {
- return this._messageText;
- }
-
- get type()
- {
- return this._type;
- }
-
- get url()
- {
- return this._url;
- }
-
- get line()
- {
- return this._line;
- }
-
- get column()
- {
- return this._column;
- }
-
- get repeatCount()
- {
- return this._repeatCount;
- }
-
- get parameters()
- {
- return this._parameters;
- }
-
- get stackTrace()
- {
- return this._stackTrace;
- }
-
- get request()
+ get source() { return this._source; }
+ get level() { return this._level; }
+ get messageText() { return this._messageText; }
+ get type() { return this._type; }
+ get url() { return this._url; }
+ get line() { return this._line; }
+ get column() { return this._column; }
+ get repeatCount() { return this._repeatCount; }
+ get parameters() { return this._parameters; }
+ get stackTrace() { return this._stackTrace; }
+ get request() { return this._request; }
+
+ get sourceCodeLocation()
{
- return this._request;
+ if (this._sourceCodeLocation !== undefined)
+ return this._sourceCodeLocation;
+
+ // First try to get the location from the top frame of the stack trace.
+ let topCallFrame = this._stackTrace.callFrames[0];
+ if (topCallFrame && topCallFrame.sourceCodeLocation) {
+ this._sourceCodeLocation = topCallFrame.sourceCodeLocation;
+ return this._sourceCodeLocation;
+ }
+
+ // If that doesn't exist try to get a location from the url/line/column in the ConsoleMessage.
+ // FIXME <http://webkit.org/b/76404>: Remove the string equality checks for undefined once we don't get that value anymore.
+ if (this._url && this._url !== "undefined") {
+ let sourceCode = WebInspector.frameResourceManager.resourceForURL(this._url);
+ if (sourceCode) {
+ let lineNumber = this._line > 0 ? this._line - 1 : 0;
+ let columnNumber = this._column > 0 ? this._column - 1 : 0;
+ this._sourceCodeLocation = new WebInspector.SourceCodeLocation(sourceCode, lineNumber, columnNumber);
+ return this._sourceCodeLocation;
+ }
+ }
+
+ this._sourceCodeLocation = null;
+ return this._sourceCodeLocation;
}
};
/*
- * Copyright (C) 2013 Apple Inc. All rights reserved.
+ * Copyright (C) 2013, 2016 Apple Inc. All rights reserved.
*
* Redistribution and use in source and binary forms, with or without
* modification, are permitted provided that the following conditions
WebInspector.IssueMessage = class IssueMessage extends WebInspector.Object
{
- constructor(source, level, text, url, lineNumber, columnNumber, parameters)
+ constructor(consoleMessage)
{
super();
- this._level = level;
- this._text = text;
- this._source = source;
+ console.assert(consoleMessage instanceof WebInspector.ConsoleMessage);
- // FIXME <http://webkit.org/b/76404>: Remove the string equality checks for undefined
- // once we don't get that value anymore from WebCore.
+ this._consoleMessage = consoleMessage;
- // FIXME: If the URL is undefined, get the URL from the stacktrace.
- if (url && url !== "undefined")
- this._url = url;
+ this._text = this._issueText();
- if (typeof lineNumber === "number" && lineNumber >= 0 && this._url)
- this._sourceCodeLocation = new WebInspector.SourceCodeLocation(WebInspector.frameResourceManager.resourceForURL(url), lineNumber, columnNumber);
-
- // FIXME: <https://webkit.org/b/142553> Web Inspector: Merge IssueMessage/ConsoleMessage - both attempt to modify the Console Messages parameter independently
-
- if (parameters && parameters !== "undefined") {
- this._parameters = [];
- for (var i = 0; i < parameters.length; ++i) {
- if (parameters[i] instanceof WebInspector.RemoteObject) {
- this._parameters.push(parameters[i]);
- continue;
- }
-
- if (typeof parameters[i] === "object")
- this._parameters.push(WebInspector.RemoteObject.fromPayload(parameters[i]));
- else
- this._parameters.push(WebInspector.RemoteObject.fromPrimitiveValue(parameters[i]));
- }
- }
-
- this._formatTextIfNecessary();
-
- switch (source) {
+ switch (this._consoleMessage.source) {
case "javascript":
// FIXME: It would be nice if we had this information (the specific type of JavaScript error)
// as part of the data passed from WebCore, instead of having to determine it ourselves.
this._type = WebInspector.IssueMessage.Type.OtherIssue;
}
+ this._sourceCodeLocation = consoleMessage.sourceCodeLocation;
if (this._sourceCodeLocation)
this._sourceCodeLocation.addEventListener(WebInspector.SourceCodeLocation.Event.DisplayLocationChanged, this._sourceCodeLocationDisplayLocationChanged, this);
}
// Public
- get type()
- {
- return this._type;
- }
-
- get level()
- {
- return this._level;
- }
-
- get text()
- {
- return this._text;
- }
-
- get source()
- {
- return this._source;
- }
-
- get url()
- {
- return this._url;
- }
-
- get lineNumber()
- {
- if (this._sourceCodeLocation)
- return this._sourceCodeLocation.lineNumber;
- }
-
- get columnNumber()
- {
- if (this._sourceCodeLocation)
- return this._sourceCodeLocation.columnNumber;
- }
+ get text() { return this._text; }
+ get type() { return this._type; }
+ get level() { return this._consoleMessage.level; }
+ get source() { return this._consoleMessage.source; }
+ get url() { return this._consoleMessage.url; }
+ get sourceCodeLocation() { return this._sourceCodeLocation; }
- get displayLineNumber()
- {
- if (this._sourceCodeLocation)
- return this._sourceCodeLocation.displayLineNumber;
- }
-
- get displayColumnNumber()
- {
- if (this._sourceCodeLocation)
- return this._sourceCodeLocation.displayColumnNumber;
- }
-
- get sourceCodeLocation()
- {
- return this._sourceCodeLocation;
- }
+ // Protected
saveIdentityToCookie(cookie)
{
cookie[WebInspector.IssueMessage.URLCookieKey] = this.url;
- cookie[WebInspector.IssueMessage.LineNumberCookieKey] = this.sourceCodeLocation.lineNumber;
- cookie[WebInspector.IssueMessage.ColumnNumberCookieKey] = this.sourceCodeLocation.columnNumber;
+ cookie[WebInspector.IssueMessage.LineNumberCookieKey] = this._sourceCodeLocation ? this._sourceCodeLocation.lineNumber : 0;
+ cookie[WebInspector.IssueMessage.ColumnNumberCookieKey] = this._sourceCodeLocation ? this._sourceCodeLocation.columnNumber : 0;
}
// Private
- _formatTextIfNecessary()
+ _issueText()
{
- if (!this._parameters)
- return;
+ let parameters = this._consoleMessage.parameters;
+ if (!parameters)
+ return this._consoleMessage.messageText;
- if (WebInspector.RemoteObject.type(this._parameters[0]) !== "string")
- return;
+ if (WebInspector.RemoteObject.type(parameters[0]) !== "string")
+ return this._consoleMessage.messageText;
function valueFormatter(obj)
{
return obj.description;
}
- var formatters = {};
+ let formatters = {};
formatters.o = valueFormatter;
formatters.s = valueFormatter;
formatters.f = valueFormatter;
return a;
}
- var result = String.format(this._parameters[0].description, this._parameters.slice(1), formatters, "", append);
- var resultText = result.formattedResult;
+ let result = String.format(parameters[0].description, parameters.slice(1), formatters, "", append);
+ let resultText = result.formattedResult;
- for (var i = 0; i < result.unusedSubstitutions.length; ++i)
+ for (let i = 0; i < result.unusedSubstitutions.length; ++i)
resultText += " " + result.unusedSubstitutions[i].description;
- this._text = resultText;
+ return resultText;
}
_sourceCodeLocationDisplayLocationChanged(event)
messageAdded(message)
{
- if (message.type === "assert" && !message.text)
- message.text = WebInspector.UIString("Assertion");
-
- if (message.level === "warning" || message.level === "error") {
- // FIXME: <https://webkit.org/b/142553> Web Inspector: Merge IssueMessage/ConsoleMessage - both attempt to modify the Console Messages parameter independently
- WebInspector.issueManager.issueWasAdded(message.source, message.level, message.text, message.url, message.line, message.column || 0);
- }
-
if (message.source === "console-api" && message.type === "clear")
return;
+ if (message.type === "assert" && !message.text)
+ message.text = WebInspector.UIString("Assertion");
+
WebInspector.logManager.messageWasAdded(message.source, message.level, message.text, message.type, message.url, message.line, message.column || 0, message.repeatCount, message.parameters, message.stackTrace, message.networkRequestId);
}
return new WebInspector.HeapAllocationsTimelineView(representedObject, extraArguments);
}
- if (representedObject instanceof WebInspector.Breakpoint) {
+ if (representedObject instanceof WebInspector.Breakpoint || representedObject instanceof WebInspector.IssueMessage) {
if (representedObject.sourceCodeLocation)
return WebInspector.ContentView.createFromRepresentedObject(representedObject.sourceCodeLocation.displaySourceCode, extraArguments);
}
if (representedObject instanceof WebInspector.Frame)
return representedObject.mainResource;
- if (representedObject instanceof WebInspector.Breakpoint) {
+ if (representedObject instanceof WebInspector.Breakpoint || representedObject instanceof WebInspector.IssueMessage) {
if (representedObject.sourceCodeLocation)
return representedObject.sourceCodeLocation.displaySourceCode;
}
return true;
if (representedObject instanceof WebInspector.Timeline)
return true;
- if (representedObject instanceof WebInspector.Breakpoint)
+ if (representedObject instanceof WebInspector.Breakpoint || representedObject instanceof WebInspector.IssueMessage)
return representedObject.sourceCodeLocation;
if (representedObject instanceof WebInspector.DOMStorageObject)
return true;
_updateTitles()
{
- var displayLineNumber = this._issueMessage.displayLineNumber;
- var displayColumnNumber = this._issueMessage.displayColumnNumber;
+ var displayLineNumber = this._issueMessage.sourceCodeLocation.displayLineNumber;
+ var displayColumnNumber = this._issueMessage.sourceCodeLocation.displayColumnNumber;
var title;
if (displayColumnNumber > 0)
title = WebInspector.UIString("Line %d:%d").format(displayLineNumber + 1, displayColumnNumber + 1); // The user visible line and column numbers are 1-based.
console.assert(!this.managesOwnIssues);
var issue = event.data.issue;
-
- // FIXME: Check more than just the issue URL (the document could have multiple resources with the same URL).
- if (issue.url !== this.resource.url)
+ if (!WebInspector.IssueManager.issueMatchSourceCode(issue, this.resource))
return;
this.addIssue(issue);
return false;
}
- _matchesIssue(issue)
- {
- if (this._sourceCode instanceof WebInspector.Resource)
- return issue.url === this._sourceCode.url;
- // FIXME: Support issues for Scripts based on id, not only by URL.
- if (this._sourceCode instanceof WebInspector.Script)
- return issue.url === this._sourceCode.url;
- return false;
- }
-
_issueWasAdded(event)
{
var issue = event.data.issue;
- if (!this._matchesIssue(issue))
+ if (!WebInspector.IssueManager.issueMatchSourceCode(issue, this._sourceCode))
return;
this._addIssue(issue);
_addIssue(issue)
{
- // FIXME: Issue should have a SourceCodeLocation.
- var sourceCodeLocation = this._sourceCode.createSourceCodeLocation(issue.lineNumber, issue.columnNumber);
+ var sourceCodeLocation = issue.sourceCodeLocation;
var lineNumber = sourceCodeLocation.formattedLineNumber;
var lineNumberIssues = this._issuesLineNumberMap.get(lineNumber);
// Avoid displaying duplicate issues on the same line.
for (var existingIssue of lineNumberIssues) {
- if (existingIssue.columnNumber === issue.columnNumber && existingIssue.text === issue.text)
+ if (existingIssue.sourceCodeLocation.columnNumber === sourceCodeLocation.columnNumber && existingIssue.text === issue.text)
return;
}
this._clearWidgets();
var issues = WebInspector.issueManager.issuesForSourceCode(this._sourceCode);
- for (var issue of issues) {
- console.assert(this._matchesIssue(issue));
+ for (var issue of issues)
this._addIssue(issue);
- }
}
_debuggerDidPause(event)
git://git.webkit.org / WebKit-https.git / commitdiff