Summary: | Web Inspector: Quickly Open to line/column does should have caret indicating where the position is | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Joseph Pecoraro <joepeck> | ||||||
Component: | Web Inspector | Assignee: | Devin Rousso <hi> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Normal | CC: | commit-queue, hi, inspector-bugzilla-changes, joepeck, mattbaker, webkit-bug-importer | ||||||
Priority: | P2 | Keywords: | InRadar | ||||||
Version: | WebKit Nightly Build | ||||||||
Hardware: | All | ||||||||
OS: | All | ||||||||
See Also: | https://bugs.webkit.org/show_bug.cgi?id=189048 | ||||||||
Bug Depends on: | |||||||||
Bug Blocks: | 190862 | ||||||||
Attachments: |
|
Description
Joseph Pecoraro
2018-10-16 15:12:56 PDT
Created attachment 352552 [details]
Patch
Created attachment 352553 [details]
Patch
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? (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. Comment on attachment 352553 [details]
Patch
r=me
Comment on attachment 352553 [details] Patch Clearing flags on attachment: 352553 Committed r237232: <https://trac.webkit.org/changeset/237232> All reviewed patches have been landed. Closing bug. |