⌘E and ⌘G do not work in main content area when quick console drawer is open Steps to reproduce: 1. Inspect webkit.org 2. Show the main resource's content in the Resources tab 3. Expand the quick console 4. Double click to select "stylesheet" in the main resource content area 5. ⌘E and ⌘G => Expected search forward, nothing happened Notes: - Both the Main ContentBrowser and the QuickConsole's ContentBrowser have a FindBanner - Both FindBanners have "enabled" their ⌘E/⌘G handling - The QuickConsole's happens first - KeyboardShortcut's global handler runs only 1 handler and bails
Possible solutions: 1. KeyboardShortcuts should specify if they have handled / if they stop propagation 2. Move away from multiple KeyboardShortcuts and behave like ⌘K where it is a global entry point and the current / active content view attempts to handle the operation. I like (2) better, since it avoid any conflict. The user has a keyboard shortcut, and they expect it to apply to the active area.
Created attachment 319963 [details] [PATCH] Proposed Fix
This is a great example of something that is not possible to test given our current test harness setup.
Comment on attachment 319963 [details] [PATCH] Proposed Fix View in context: https://bugs.webkit.org/attachment.cgi?id=319963&action=review r=me, Thank you Joe! > Source/WebInspectorUI/ChangeLog:45 > + are ContentBrowsers Nit: missing end of sentence > Source/WebInspectorUI/UserInterface/Base/Main.js:291 > + // FIXME: FindBanner search queries should be shared among search fields and be common with the system. Nit: you are looking for bugs 151310 and 113588. > Source/WebInspectorUI/UserInterface/Views/ContentView.js:452 > + let selection = window.getSelection(); I guess this is better than nothing. We need to do a better job overriding this in more ContentView subclasses. > Source/WebInspectorUI/UserInterface/Views/FindBanner.js:-227 > - { I didn't realize we were doing this, its kinda gross. If we really were going to do this in a transparent way that each content view handles it by itself, then we'd need some notion of a first responder and responder chain. I think the solution in this patch is fine, though.
Created attachment 320046 [details] [PATCH] For Landing
Comment on attachment 320046 [details] [PATCH] For Landing Clearing flags on attachment: 320046 Committed r221691: <http://trac.webkit.org/changeset/221691>
<rdar://problem/34693554>