Bug 160840 – Web Inspector: Entering ":n" in Open Resource Dialog, where n > number of lines, should jump to the last line

• Component: Web Inspector
• Status: RESOLVED FIXED

[Nikita Vasilyev]

Steps:
1. Open Inspector on this page.
2. Press Command-Shift-O
3. Type "global.js:99999999"

Expected:
global.js opens and focused on the last line.

Actual:
global.js opens and focuses on the previously focused line (or just scrolls to the top of the resource).

Notes:
This is how it works in Sublime Text and Chrome DevTools.

[Radar WebKit Bug Importer]

<rdar://problem/27835331>

[Devin Rousso]

Created attachment 286440 [details]
Patch

[Joseph Pecoraro]

Comment on attachment 286440 [details]
Patch

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

> Source/WebInspectorUI/UserInterface/Views/TextEditor.js:435
> +        let line = Number.constrain(position.lineNumber, 0, this._codeMirror.lineCount() - 1);
> +        let column = Number.constrain(position.columnNumber, 0, this._codeMirror.getLine(line).length - 1);
> +
> +        let lineHandle = this._codeMirror.getLineHandle(line);
>          if (!lineHandle || !this._visible || this._initialStringNotSet || this._deferReveal) {

How does this handle editors that haven't yet loaded content (initial string not set, defer reveal)?

Is lineCount() zero, and so line would be -1, and lineHandle would not exist?

On the surface this patch seems fine, but I worry that if "deferReveal" is true, we would have just lost the line number.

The test scenario for this is click a link to reveal a position in a resource that will be automatically pretty printed.

[Devin Rousso]

Created attachment 286530 [details]
Patch

[Joseph Pecoraro]

Comment on attachment 286530 [details]
Patch

This feels right! I'll review more closely when at a computer.

[Joseph Pecoraro]

Comment on attachment 286530 [details]
Patch

r=me, I like this much better. Thanks for addressing the comments.

To be sure, did you test with a an auto-formatted resource? Let me know if you have questions.

[WebKit Commit Bot]

Comment on attachment 286530 [details]
Patch

Clearing flags on attachment: 286530

Committed r204755: <http://trac.webkit.org/changeset/204755>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Nikita Vasilyev]

This isn't working for me on Safari Technology Preview 16.

[Devin Rousso]

Created attachment 293245 [details]
Patch

[WebKit Commit Bot]

Comment on attachment 293245 [details]
Patch

Clearing flags on attachment: 293245

Committed r208188: <http://trac.webkit.org/changeset/208188>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.