WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
16313
text search (find) keybindings should work in the Web Inspector
https://bugs.webkit.org/show_bug.cgi?id=16313
Summary
text search (find) keybindings should work in the Web Inspector
Rachael Worthington (cheers)
Reported
2007-12-05 15:13:19 PST
Current behavior: hitting cmd-F gives an error bonk, cmd-G does nothing desired behavior: hitting cmd-F should either bring up a find panel, for searching just the file you're focused on, or at least move your cursor to the find field in the upper right so that you can enter search terms. cmd-G and cmd-shift-G should cycle forwards and backwards through search results like it does elsewhere. (This is something of a pet peeve of mine)
Attachments
Patch for Cmd-F or Ctrl-F
(1.42 KB, patch)
2008-07-22 01:03 PDT
,
Anthony Ricaud
no flags
Details
Formatted Diff
Diff
Same as original but endeavors to avoid false-positive keystrokes...
(1.78 KB, patch)
2008-07-27 00:44 PDT
,
Daniel Jalkut
no flags
Details
Formatted Diff
Diff
Same as original + avoid false positives + style edits via bdash
(1.73 KB, patch)
2008-07-27 01:43 PDT
,
Daniel Jalkut
no flags
Details
Formatted Diff
Diff
Add credit/blame line in case there are questions or requests for fixes from review.
(1.85 KB, patch)
2008-07-27 10:39 PDT
,
Daniel Jalkut
no flags
Details
Formatted Diff
Diff
Ugh - learn to spell my email addr.
(1.85 KB, patch)
2008-07-27 10:43 PDT
,
Daniel Jalkut
timothy
: review+
Details
Formatted Diff
Diff
Show Obsolete
(4)
View All
Add attachment
proposed patch, testcase, etc.
Adam Roben (:aroben)
Comment 1
2008-01-29 11:07:18 PST
<
rdar://problem/5712878
>
Anthony Ricaud
Comment 2
2008-07-22 01:03:34 PDT
Created
attachment 22419
[details]
Patch for Cmd-F or Ctrl-F I'm not sure the early break; is a good idea for code readability. I'll post a patch for Cmd+G and Cmd-Shift-G later.
Daniel Jalkut
Comment 3
2008-07-26 23:04:59 PDT
I stumbled on this bug because it was referenced in IRC. The only skepticism that jumps to my mind is whether the "match" is too inclusive. For instance, this patch is intended to allow ctrl-F and cmd-F to perform a certain action, but the code as written will (I think) allow many key combinations to perform this action: cmd-ctrl-F cmd-shift-F ctrl-shift-F opt-ctrl-F Etc. I don't know enough about the code flow around this to know whether this matters. But generally when keyboard shortcut modifiers are being explicitly tested, it's important to confirm that the desired modifier is being pressed TO THE EXCLUSION of the other modifier keys.
Daniel Jalkut
Comment 4
2008-07-27 00:44:25 PDT
Created
attachment 22500
[details]
Same as original but endeavors to avoid false-positive keystrokes... I confirmed my suspicion that the original patch made it possible to invoke the find command by a number of undesired keystrokes. With this adjusted patch, the keystroke only invokes find if it's a pure cmd-f or ctrl-f.
Daniel Jalkut
Comment 5
2008-07-27 01:43:31 PDT
Created
attachment 22502
[details]
Same as original + avoid false positives + style edits via bdash Fixes style mistakes I made. Props to bdash for noticing/informing.
Daniel Jalkut
Comment 6
2008-07-27 01:44:56 PDT
Comment on
attachment 22500
[details]
Same as original but endeavors to avoid false-positive keystrokes... Obsoleted by later patch.
Daniel Jalkut
Comment 7
2008-07-27 10:39:45 PDT
Created
attachment 22506
[details]
Add credit/blame line in case there are questions or requests for fixes from review.
Daniel Jalkut
Comment 8
2008-07-27 10:43:42 PDT
Created
attachment 22507
[details]
Ugh - learn to spell my email addr.
mitz
Comment 9
2008-07-27 10:49:02 PDT
Comment on
attachment 22506
[details]
Add credit/blame line in case there are questions or requests for fixes from review. Is it okay to hard-code 'F' in inspector.js? How will it work with non-US keyboard layouts or with localizations where the keyboard shortcut for Find is not Command-F?
Daniel Jalkut
Comment 10
2008-07-27 10:53:06 PDT
Perhaps a better long-term solution is to arrange for the Inspector to react to the existing menu items of the app. First-responder style. For instance, it's kind of dumb that "Cmd-F" will now work to activate the find field, yet Edit -> Find is disabled in the menu bar. This also touches on another issue that a friend noticed, where he wanted a "printable" version of the inspector window. If "Print" from the hosting app's menu was hooked up to the inspector, it might "just work" for some definition of working. Daniel
Timothy Hatcher
Comment 11
2008-08-01 15:20:22 PDT
Comment on
attachment 22507
[details]
Ugh - learn to spell my email addr. We should find a way to hook up to the menu, for sure. But I think this is fine for now. We might also have different find in the future, like a current file find (Command-F and Command-G). Then the global search, so the shortcut might change to mean local find in the future.
Timothy Hatcher
Comment 12
2008-08-15 10:03:10 PDT
Landed in
r35785
.
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