Bug 45166 – Web Inspector: implement go-to-line feature

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

[Graham Ballantyne]

Problem: the web inspector lacks a go-to-line feature in the Scripts & Resources panes.

Steps to reproduce:

1. Open the web inspector.
2. View a script in the Scripts pane.

Desired outcome: invoke the nifty go-to-line feature, enter a line number and be magically whisked off to your specified line. Dance a little happy dance of joy.

Actual outcome: look around for the nifty go-to-line feature and find none. Resort to doing it how our father's fathers did it: scrolling your 15 kiloline script by hand, in the snow, uphill both ways. No dancing.

Having a keyboard shortcut would be good, but there isn't an obvious one (CMD-L/CTRL-L is taken, as is CMD-OPT-L/insert-windows-equivalent). CMD-SHIFT-L?

[Joseph Pecoraro]

Thanks for filing the bug, with a little bit of humor too. It'll be a challenge
coming up with a discoverable UI solution to this.

[Yury Semikhatsky]

Created attachment 67910 [details]
Patch

[Yury Semikhatsky]

Created attachment 67911 [details]
Safari screenshot for the patch

[Ilya Tikhonovsky]

It'd be better to have shortcuts for this.
Visual Studio uses Ctrl-g
Eclipse uses Ctrl-L
XCode uses Cmd-L
InteliJ Idea ?

Also some button is required for mouse men.

[Yury Semikhatsky]

Created attachment 67914 [details]
Chromium screenshot

[Yury Semikhatsky]

(In reply to comment #4)
> It'd be better to have shortcuts for this.
> Visual Studio uses Ctrl-g
> Eclipse uses Ctrl-L
> XCode uses Cmd-L

This is already used for jump to location bar I'm not sure we can easily override this in Web Inspector.


> InteliJ Idea ?

It's  Ctrl+g there.


> Also some button is required for mouse men.
Well, I'd rather keep it as simple as it's now but given that there are already two of you who wants to see a pair of Ok/Cancel buttons I'm ready to add them.

Timothy, any criticism/thoughts on how this dialog should look like?

[Joseph Pecoraro]

Comment on attachment 67910 [details]
Patch

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

Neat solution Yury! A few observations from the screenshots.

> WebCore/inspector/front-end/GoToLineDialog.js:45
> +    labelText = WebInspector.UIString("Go to line%s:", labelText);

I'd like to see a few more space spaces here:

  "Go to line (1-300): [            ]"
             ^        ^

> WebCore/inspector/front-end/GoToLineDialog.js:97
> +    },

I don't know if we typically include the extra comma here. Its fine
by me, it just caught my eye.

[Joseph Pecoraro]

Whoops, I got so carried away with the new review features I forgot my main point.
I think the <input> could be narrower. No need to have it be so wide, if the usual
input is going to nearly always be a number less than 5 characters wide.

[Yury Semikhatsky]

Created attachment 68888 [details]
Safari screenshot(narrower text field)

Centered the window, added spaces, reduced input width. Perdonally I think that overall look of a wider text field is better, even though user may never need such a long text input.

[Yury Semikhatsky]

Created attachment 68893 [details]
Chromium screenshot - 2

[Yury Semikhatsky]

Created attachment 68894 [details]
Patch

[Pavel Feldman]

Comment on attachment 68894 [details]
Patch

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

Please add this to VC project as well.

> WebCore/inspector/front-end/GoToLineDialog.js:31
> +WebInspector.GoToLineDialog = function(view)

It'd nice to factor it out into generic dialog.

> WebCore/inspector/front-end/GoToLineDialog.js:44
> +    var labelText = linesCount ? " (1 - " + linesCount + ")" : "";

Should be localizable.

[Yury Semikhatsky]

Created attachment 68908 [details]
Patch

[Yury Semikhatsky]

Created attachment 68909 [details]
Safari screenshot(with go button)

[Yury Semikhatsky]

(In reply to comment #7)
> (From update of attachment 67910 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=67910&action=prettypatch
> 
> Neat solution Yury! A few observations from the screenshots.
> 
> > WebCore/inspector/front-end/GoToLineDialog.js:45
> > +    labelText = WebInspector.UIString("Go to line%s:", labelText);
> 
> I'd like to see a few more space spaces here:
> 
>   "Go to line (1-300): [            ]"
>              ^        ^
> 
Done.


> > WebCore/inspector/front-end/GoToLineDialog.js:97
> > +    },
> 
> I don't know if we typically include the extra comma here. Its fine
> by me, it just caught my eye.
Fixed.

(In reply to comment #8)
> Whoops, I got so carried away with the new review features I forgot my main point.
> I think the <input> could be narrower. No need to have it be so wide, if the usual
> input is going to nearly always be a number less than 5 characters wide.
Done. The input width is 6 characters.


(In reply to comment #12)
> (From update of attachment 68894 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=68894&action=review
> 
> Please add this to VC project as well.
> 
Done.

> > WebCore/inspector/front-end/GoToLineDialog.js:31
> > +WebInspector.GoToLineDialog = function(view)
> 
> It'd nice to factor it out into generic dialog.
> 
I'd rather wait until its code is stable enough and we know what we want to see as the generic dialog.

> > WebCore/inspector/front-end/GoToLineDialog.js:44
> > +    var labelText = linesCount ? " (1 - " + linesCount + ")" : "";
> 
> Should be localizable.
Done.

[Yury Semikhatsky]

Created attachment 68910 [details]
Chromium screenshot - 3(with go button)

[Timothy Hatcher]

The overall look is nice for the new screenshots. But the (1-2344) part is not the best design. I think a seperate, smaller and grey, label under the text field (or just as a tooltip) would be best. You can clamp the entered line to the max line, so typing 999999 would get you the last line without needing to read or think. So the info isn't that useful if you clamp.

[Yury Semikhatsky]

Created attachment 68914 [details]
Patch

[Yury Semikhatsky]

(In reply to comment #17)
> The overall look is nice for the new screenshots. But the (1-2344) part is not the best design. I think a seperate, smaller and grey, label under the text field (or just as a tooltip) would be best. You can clamp the entered line to the max line, so typing 999999 would get you the last line without needing to read or think. So the info isn't that useful if you clamp.

Added clamping and moved "1 - n" into a tooltip as we agreed on the IRC channel.

[Yury Semikhatsky]

Committed r68398