In almost all Cocoa applications on the Mac where you can search for something in the document, the find command is system wide. That is, you can highlight a word, press Command-E to make it the search term, then switch to another Cocoa application and press Command-G to find again. The second application will search for the term from the first application. And it goes both ways (search for a term in the second app, find it in the first app). This generally works great in Safari and Chrome, and I often use it as a developer to highlight something on a page, press command-e and then find it in BBEdit (and vice versa). Or I highlight something in an e-mail and then go find it in a Web page. However, the developer tools don't work that way. My deeply-ingrained habit of using this very useful standard Mac keyboard shortcut don't work in the developer tools, because they don't access the systemwide Find pasteboard (or whatever its called). I end up having to copy and paste the term into the Find field for "Sources" and "Resources" especially (it is what I search for things in the most), and also for "Elements". Please bring system-wide "Find" to the Inspector and Developer Tools. PS. I have "Use Webkit Inspector" checked in the "Develop" menu all the time, because I find the UI on the newer version of the inspector tools to be pretty much unusable, and a huge step backwards. I hate the new version.
More details: On Mac, doing Ctrl+E on selected content within the inspector will: * put it into the system find pasteboard (NSFindPboard?) * populate the main content view's find dialog with that text (open or closed) doing Ctrl+E on inspected page content, or any other application, will put it into the find pasteboard but not populate any find dialog in the inspector.
<rdar://problem/19281466>
Created attachment 397432 [details] Patch this has bothered me for too long T.T
Comment on attachment 397432 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=397432&action=review There are some lifetime issues to address in the ObjC parts, but other than that this patch looks god and is a welcome improvement! > Source/WebKit/UIProcess/Inspector/mac/WKInspectorViewController.mm:240 > + if (!!_delegate && [_delegate respondsToSelector:@selector(inspectorViewControllerDidBecomeActive:)]) Nit: respondsToSelector check will return NO if _delegate is nil, so the null check is not necessary. > Source/WebKit/UIProcess/Inspector/mac/WKInspectorWKWebView.mm:62 > + [[NSNotificationCenter defaultCenter] addObserver:self selector:@selector(_handleWindowDidBecomeKey:) name:NSWindowDidBecomeKeyNotification object:nil]; This needs to be unregistered at some point or `self` will be retained forever by NSNotificationCenter. > Source/WebKit/UIProcess/Inspector/mac/WKInspectorWKWebView.mm:88 > +- (BOOL)becomeFirstResponder Does this correctly handle the case when Inspector is attached? > Source/WebKit/UIProcess/Inspector/mac/WKInspectorWKWebView.mm:95 > +- (void)_handleWindowDidBecomeKey:(NSNotification *)notification You don't check which window is key, so this would probably fire if Web Inspector window is not key.
good* 😆
Comment on attachment 397432 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=397432&action=review >> Source/WebKit/UIProcess/Inspector/mac/WKInspectorWKWebView.mm:62 >> + [[NSNotificationCenter defaultCenter] addObserver:self selector:@selector(_handleWindowDidBecomeKey:) name:NSWindowDidBecomeKeyNotification object:nil]; > > This needs to be unregistered at some point or `self` will be retained forever by NSNotificationCenter. Ah, oops. >> Source/WebKit/UIProcess/Inspector/mac/WKInspectorWKWebView.mm:88 >> +- (BOOL)becomeFirstResponder > > Does this correctly handle the case when Inspector is attached? I believe so, but only if the Safari window was already focused (e.g. clicking on Web Inspector when the inspected page was previously focused). I also have the `NSWindowDidBecomeKeyNotification` to handle the case that Web Inspector is focused (as well as if it's its own window).
Created attachment 397693 [details] Patch
Created attachment 397804 [details] Patch
Comment on attachment 397804 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=397804&action=review r=me! > Source/WebKit/UIProcess/Inspector/mac/WKInspectorWKWebView.mm:98 > + [self.inspectorWKWebViewDelegate inspectorWKWebViewDidBecomeActive:self]; Nit: usually prefer to use the ivar in this situation, _inspectorWKWebViewDelegate. > Source/WebKit/UIProcess/Inspector/mac/WKInspectorWKWebView.mm:104 > + if ([notification object] == [self window]) nit: notification.object, self.window
Comment on attachment 397804 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=397804&action=review >> Source/WebKit/UIProcess/Inspector/mac/WKInspectorWKWebView.mm:98 >> + [self.inspectorWKWebViewDelegate inspectorWKWebViewDidBecomeActive:self]; > > Nit: usually prefer to use the ivar in this situation, _inspectorWKWebViewDelegate. I'd prefer to follow the convention of what is done in this file. Not to mention, the ivar is a `WeakObjCPtr<id <WKInspectorWKWebViewDelegate>>` so I think I'd have to do some extra casting, which `-[WKInspectorWKWebView inspectorWKWebViewDelegate]` does for me. >> Source/WebKit/UIProcess/Inspector/mac/WKInspectorWKWebView.mm:104 >> + if ([notification object] == [self window]) > > nit: notification.object, self.window lol oops :P
Created attachment 397875 [details] Patch
Committed r260847: <https://trac.webkit.org/changeset/260847> All reviewed patches have been landed. Closing bug and clearing flags on attachment 397875 [details].