Change App Highlight delegate to pass more information about the highlight for display.
Created attachment 420570 [details] Patch
Comment on attachment 420570 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=420570&action=review > Source/WebKit/UIProcess/API/Cocoa/_WKAppHighlight.h:37 > +- (instancetype) init NS_UNAVAILABLE; )init > Source/WebKit/UIProcess/API/Cocoa/_WKAppHighlight.h:39 > +@property (nonatomic, readonly, copy) NSData *data; I think just "data" is a bit mysterious. This is the thing that you pass to "restore", right? I think we can come up with a name. > Source/WebKit/UIProcess/API/Cocoa/_WKAppHighlight.h:49 > +@property (nonatomic, readonly) BOOL newGroup; This one doesn't make sense here and should be the second parameter on the method, I think > Source/WebKit/UIProcess/API/Cocoa/_WKAppHighlight.mm:2 > + * Copyright (C) 20201 Apple Inc. All rights reserved. Wow that's a distant future you've got there > Source/WebKit/UIProcess/API/Cocoa/_WKAppHighlightDelegate.h:36 > +- (void)_webView:(WKWebView *)webView storeAppHighlight:(_WKAppHighlight *)highlight WK_API_AVAILABLE(macos(WK_MAC_TBA), ios(WK_IOS_TBA)); I propose: _webView:didCreateHighlight:creatingNewGroup:
> _webView:didCreateHighlight:creatingNewGroup: or ... _webView:didCreateHighlight:inNewGroup:?
Comment on attachment 420570 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=420570&action=review >> Source/WebKit/UIProcess/API/Cocoa/_WKAppHighlight.h:39 >> +@property (nonatomic, readonly, copy) NSData *data; > > I think just "data" is a bit mysterious. This is the thing that you pass to "restore", right? I think we can come up with a name. I actually call it 'highlight' in much of the previous patch, but if we come up with an even better name, I'll change it everywhere.
Comment on attachment 420570 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=420570&action=review > Source/WebKit/UIProcess/API/Cocoa/WKWebView.mm:1432 > + _WKAppHighlight *wkHighlight = [[_WKAppHighlight alloc] initWithData:highlight.highlight->createNSData().get() text:text image:nil createNewGroup:highlight.isNewGroup == WebCore::CreateNewGroupForHighlight::Yes ? YES : NO]; It looks like this will leak.
Created attachment 420578 [details] Patch
Created attachment 420586 [details] Patch
Comment on attachment 420586 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=420586&action=review > Source/WebKit/UIProcess/API/Cocoa/_WKAppHighlight.mm:39 > + _text = text; Where'd the image go > Source/WebKit/UIProcess/API/Cocoa/_WKInspectorExtension.mm:29 > + Too many newlines
Created attachment 420611 [details] Patch
Created attachment 420614 [details] Patch
Comment on attachment 420614 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=420614&action=review > Source/WebKit/UIProcess/API/Cocoa/WKWebView.mm:1434 > + if ([delegate respondsToSelector:@selector(_webView:storeAppHighlight:inNewGroup:)]) You should probably just bail at the top, none of this work is worth anything if you're not going to call the delegate. > Source/WebKit/UIProcess/API/Cocoa/_WKAppHighlight.h:41 > +@property (nonatomic, readonly, copy) NSString * text; * is a-floating > Source/WebKit/UIProcess/API/Cocoa/_WKInspectorExtension.mm:29 > + This extraneous newline is still here
Comment on attachment 420614 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=420614&action=review > Source/WebKit/UIProcess/API/Cocoa/_WKAppHighlight.mm:39 > + _text = text; Where'd the image go? I think you missed my last set of comments
Created attachment 420619 [details] Patch
Created attachment 420655 [details] Patch for landing
Committed r273013: <https://commits.webkit.org/r273013> All reviewed patches have been landed. Closing bug and clearing flags on attachment 420655 [details].
<rdar://problem/74440009>