Bug 16313 – text search (find) keybindings should work in the Web Inspector

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

[Rachael Worthington (cheers)]

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)

[Adam Roben (:aroben)]

<rdar://problem/5712878>

[Anthony Ricaud]

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]

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]

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]

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 on attachment 22500 [details]
Same as original but endeavors to avoid false-positive keystrokes...

Obsoleted by later patch.

[Daniel Jalkut]

Created attachment 22506 [details]
Add credit/blame line in case there are questions or requests for fixes from review.

[Daniel Jalkut]

Created attachment 22507 [details]
Ugh - learn to spell my email addr.

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]

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 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]

Landed in r35785.