Bug 61035 – Web Inspector: hide script location to ui location conversion details from ScriptsPanel

• Status: RESOLVED
• Resolution: FIXED
• Component: Web Inspector (Deprecated)

[Pavel Podivilov]

Web Inspector: add ScriptLocation and UILocation classes and hide conversions in model.

[Pavel Podivilov]

Created attachment 93896 [details]
Patch.

[Pavel Feldman]

Comment on attachment 93896 [details]
Patch.

View in context: https://bugs.webkit.org/attachment.cgi?id=93896&action=review

> Source/WebCore/inspector/front-end/DebuggerPresentationModel.js:615
> +WebInspector.ScriptLocation = function(url, sourceID, lineNumber, columnNumber)

These should be defined near WebInspector.SourceMapping and should actually be used there.

[Pavel Podivilov]

(In reply to comment #2)
> (From update of attachment 93896 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=93896&action=review
> 
> > Source/WebCore/inspector/front-end/DebuggerPresentationModel.js:615
> > +WebInspector.ScriptLocation = function(url, sourceID, lineNumber, columnNumber)
> 
> These should be defined near WebInspector.SourceMapping and should actually be used there.

ScriptLocation has nothing to do with WebInspector.SourceMapping. SourceMapping always defines a mapping for a single script. ScriptLocation -> UILocation conversion is a part of DebuggerPresentationModel, so it should be defined there.

[Pavel Feldman]

> ScriptLocation has nothing to do with WebInspector.SourceMapping. SourceMapping always defines a mapping for a single script. ScriptLocation -> UILocation conversion is a part of DebuggerPresentationModel, so it should be defined there.

Hm... I thought that the "mapping" concept of the presentation model was extracted from it as a SourceMapping. At least all conversions from script location to ui location and back are declared on SourceMapping.

[Pavel Podivilov]

(In reply to comment #4)
> > ScriptLocation has nothing to do with WebInspector.SourceMapping. SourceMapping always defines a mapping for a single script. ScriptLocation -> UILocation conversion is a part of DebuggerPresentationModel, so it should be defined there.
> 
> Hm... I thought that the "mapping" concept of the presentation model was extracted from it as a SourceMapping. At least all conversions from script location to ui location and back are declared on SourceMapping.

SourceMapping is an dpm utility class aimed at converting locations for a single script. We still need a way to convert sourceURL,line,column to sourceFileId,line, which is different.

[Pavel Feldman]

> SourceMapping is an dpm utility class aimed at converting locations for a single script. We still need a way to convert sourceURL,line,column to sourceFileId,line, which is different.

It does not make sense to me. I think we only have ScriptLocation and UILocation and conversion between them.

[Pavel Podivilov]

Created attachment 93900 [details]
Comments addressed.

No new abstractions this time. Probably we'll add them later when there will be a more clear need.

[Pavel Podivilov]

Created attachment 93902 [details]
Rebase.

[Pavel Feldman]

Comment on attachment 93902 [details]
Rebase.

View in context: https://bugs.webkit.org/attachment.cgi?id=93902&action=review

> Source/WebCore/inspector/front-end/DebuggerPresentationModel.js:83
> +    scriptLocationToUILocation: function(sourceURL, sourceID, lineNumber, columnNumber, callback)

sourceId

> Source/WebCore/inspector/front-end/ScriptsPanel.js:526
> +                this._showSourceLine(sourceFileId, lineNumber);

We should not highlight line in case it was not specified in anchor.

[Pavel Podivilov]

Created attachment 93906 [details]
Patch.

[Pavel Podivilov]

(In reply to comment #9)
> (From update of attachment 93902 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=93902&action=review
> 
> > Source/WebCore/inspector/front-end/DebuggerPresentationModel.js:83
> > +    scriptLocationToUILocation: function(sourceURL, sourceID, lineNumber, columnNumber, callback)
> 
> sourceId

Done.

> 
> > Source/WebCore/inspector/front-end/ScriptsPanel.js:526
> > +                this._showSourceLine(sourceFileId, lineNumber);
> 
> We should not highlight line in case it was not specified in anchor.

This code is executed only when line is specified in anchor.

[Pavel Podivilov]

Committed r88344: <http://trac.webkit.org/changeset/88344>