Clients should be able to customize the view used for page previews
<rdar://problem/18889231>
Created attachment 241074 [details] Patch
Comment on attachment 241074 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=241074&action=review > Source/WebKit2/UIProcess/API/Cocoa/WKViewPrivate.h:124 > +- (NSView *)_previewViewForURL:(NSURL *)url frame:(NSRect)frame; This sounds kind of ambiguous, as both a verb and a noun. I can't think of a better name though. Can it return a WKView?
(In reply to comment #3) > Comment on attachment 241074 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=241074&action=review > > > Source/WebKit2/UIProcess/API/Cocoa/WKViewPrivate.h:124 > > +- (NSView *)_previewViewForURL:(NSURL *)url frame:(NSRect)frame; > > This sounds kind of ambiguous, as both a verb and a noun. I can't think of a > better name though. > Can it return a WKView? I'd rather not constrain it as such, especially since the internal implementation actually uses WKWebView. I initially had -_viewForPreviewingURL:frame:, but thought this read better. Perhaps I was wrong?
Comment on attachment 241074 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=241074&action=review > Source/WebKit2/UIProcess/mac/WKActionMenuController.mm:112 > + RetainPtr<NSView> previewView; > + NSRect defaultFrame = NSMakeRect(0, 0, _mainViewSize.width, _mainViewSize.height); I'd move these around, put the initial assignment here and then have the check below be if (!previewView) { ... } > Source/WebKit2/UIProcess/mac/WKActionMenuController.mm:819 > +- (RetainPtr<NSView>)pagePreviewViewController:(WKPagePreviewViewController *)pagePreviewViewController previewViewForURL:(NSURL *)url frame:(NSRect)frame I don't think this needs to return a RetainPtr.
(In reply to comment #5) > Comment on attachment 241074 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=241074&action=review > > > Source/WebKit2/UIProcess/mac/WKActionMenuController.mm:112 > > + RetainPtr<NSView> previewView; > > + NSRect defaultFrame = NSMakeRect(0, 0, _mainViewSize.width, _mainViewSize.height); > > I'd move these around, put the initial assignment here and then have the > check below be > > if (!previewView) { > ... > } > Agreed, will fix. > > Source/WebKit2/UIProcess/mac/WKActionMenuController.mm:819 > > +- (RetainPtr<NSView>)pagePreviewViewController:(WKPagePreviewViewController *)pagePreviewViewController previewViewForURL:(NSURL *)url frame:(NSRect)frame > > I don't think this needs to return a RetainPtr. I don't think so either.
Created attachment 241113 [details] Patch
Comment on attachment 241113 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=241113&action=review > Source/WebKit2/UIProcess/API/Cocoa/WKViewPrivate.h:124 > +- (NSView *)_viewForPreviewingURL:(NSURL *)url frame:(NSRect)frame; I think this should be called initialFrameSize (and take an NSSize).
(In reply to comment #8) > Comment on attachment 241113 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=241113&action=review > > > Source/WebKit2/UIProcess/API/Cocoa/WKViewPrivate.h:124 > > +- (NSView *)_viewForPreviewingURL:(NSURL *)url frame:(NSRect)frame; > > I think this should be called initialFrameSize (and take an NSSize). Agreed; new version coming.
Created attachment 241147 [details] Patch
Comment on attachment 241147 [details] Patch Clearing flags on attachment: 241147 Committed r175732: <http://trac.webkit.org/changeset/175732>
All reviewed patches have been landed. Closing bug.