WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
190643
Web Inspector: Quickly Open to line/column does should have caret indicating where the position is
https://bugs.webkit.org/show_bug.cgi?id=190643
Summary
Web Inspector: Quickly Open to line/column does should have caret indicating ...
Joseph Pecoraro
Reported
2018-10-16 15:12:56 PDT
Quickly Open to line/column does should have caret indicating where the position is Steps to Reproduce: 1. Inspect <
http://bogojoker.com/shell/
> 2. Show the "easySlider.min.js" resource in the Debugger tab 3. Unformat the resource so it is not pretty printed 4. ⌘⇧O to trigger the quickly open dialog 5. Type ":5:507" and press return to go to a specific line/column => The editor moves and flashes, but it is unclear where the exact position is, a caret should be showing to indicate it
Attachments
Patch
(2.96 KB, patch)
2018-10-16 23:24 PDT
,
Devin Rousso
no flags
Details
Formatted Diff
Diff
Patch
(3.10 KB, patch)
2018-10-16 23:24 PDT
,
Devin Rousso
no flags
Details
Formatted Diff
Diff
Show Obsolete
(1)
View All
Add attachment
proposed patch, testcase, etc.
Devin Rousso
Comment 1
2018-10-16 23:24:31 PDT
Created
attachment 352552
[details]
Patch
Devin Rousso
Comment 2
2018-10-16 23:24:57 PDT
Created
attachment 352553
[details]
Patch
Matt Baker
Comment 3
2018-10-17 10:27:16 PDT
Comment on
attachment 352553
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=352553&action=review
This solves the problem, but I'm a little wary of changing revealPosition to force the editor to become focused. We call revealPosition in about a dozen places, are we sure this is the behavior we want?
> Source/WebInspectorUI/UserInterface/Views/SourceCodeTextEditor.js:-284 > - this.focus();
Why not just move this above the call to revealPosition? See comment below.
> Source/WebInspectorUI/UserInterface/Views/TextEditor.js:231 > + this.focus();
Is there ever a time we would want to set the selection, but not focus the editor?
Joseph Pecoraro
Comment 4
2018-10-17 11:50:02 PDT
(In reply to Matt Baker from
comment #3
)
> We call revealPosition in about a dozen places, are we sure this is the behavior we want?
This sounds like a good idea to me. We could land it, and live on it, and see if there are any cases that stand out. I imagine the most likely issue I'd come across is that `Escape` in an editable TextEditor might not take me to the QuickConsole due to focus being inside the TextEditor. I think I might be okay with that, and we might even be able to improve that situation if it even happens at all.
Matt Baker
Comment 5
2018-10-17 12:22:36 PDT
Comment on
attachment 352553
[details]
Patch r=me
WebKit Commit Bot
Comment 6
2018-10-17 12:47:01 PDT
Comment on
attachment 352553
[details]
Patch Clearing flags on attachment: 352553 Committed
r237232
: <
https://trac.webkit.org/changeset/237232
>
WebKit Commit Bot
Comment 7
2018-10-17 12:47:02 PDT
All reviewed patches have been landed. Closing bug.
Radar WebKit Bug Importer
Comment 8
2018-10-17 12:48:49 PDT
<
rdar://problem/45347635
>
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug