WebKit should manage its own view for the Quick Look menu item.
<rdar://problem/18871842>
Created attachment 240948 [details] Patch
Comment on attachment 240948 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=240948&action=review > Source/WebKit2/UIProcess/mac/WKActionMenuController.mm:70 > +@interface _WKPagePreviewViewController : NSViewController { This does not be prefixed by an underscore since it's an internal class.
Comment on attachment 240948 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=240948&action=review > Source/WebKit2/UIProcess/mac/WKActionMenuController.mm:71 > + @private; don't think that semicolon belongs, does it? > Source/WebKit2/UIProcess/mac/WKActionMenuController.mm:91 > + _preferredSize = NSMakeSize(320, 568); Where do these crazy numbers come from! > Source/WebKit2/UIProcess/mac/WKActionMenuController.mm:223 > + NSPopover *popover = [[NSPopover alloc] init]; is the memory management of popover correct?
(In reply to comment #3) > Comment on attachment 240948 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=240948&action=review > > > Source/WebKit2/UIProcess/mac/WKActionMenuController.mm:70 > > +@interface _WKPagePreviewViewController : NSViewController { > > This does not be prefixed by an underscore since it's an internal class. Removed.
(In reply to comment #4) > Comment on attachment 240948 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=240948&action=review > > > Source/WebKit2/UIProcess/mac/WKActionMenuController.mm:71 > > + @private; > > don't think that semicolon belongs, does it? Nope, not sure how that snuck in. > > > Source/WebKit2/UIProcess/mac/WKActionMenuController.mm:91 > > + _preferredSize = NSMakeSize(320, 568); > > Where do these crazy numbers come from! "somewhat arbitrary" = "iPhone 5 screen size"; I'll add that to the ChangeLog. > > > Source/WebKit2/UIProcess/mac/WKActionMenuController.mm:223 > > + NSPopover *popover = [[NSPopover alloc] init]; > > is the memory management of popover correct? No, that should use RetainPtr too. New patch coming.
Created attachment 240959 [details] Patch
Created attachment 240961 [details] Patch
Comment on attachment 240961 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=240961&action=review > Source/WebKit2/UIProcess/mac/WKActionMenuController.mm:71 > + @private I don't think this needs to be indented. > Source/WebKit2/UIProcess/mac/WKActionMenuController.mm:77 > +@property (nonatomic, readonly) NSURL *url; > +@property (nonatomic) NSSize preferredSize; I think these can just be ivars, they don't need to be properties.
(In reply to comment #9) > Comment on attachment 240961 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=240961&action=review > > > Source/WebKit2/UIProcess/mac/WKActionMenuController.mm:71 > > + @private > > I don't think this needs to be indented. Fixed. > > > Source/WebKit2/UIProcess/mac/WKActionMenuController.mm:77 > > +@property (nonatomic, readonly) NSURL *url; > > +@property (nonatomic) NSSize preferredSize; > > I think these can just be ivars, they don't need to be properties. It feels really strange to me to prefer direct ivar access, but I'll post a new version doing so.
Created attachment 240971 [details] Patch
Comment on attachment 240971 [details] Patch Clearing flags on attachment: 240971 Committed r175589: <http://trac.webkit.org/changeset/175589>
All reviewed patches have been landed. Closing bug.
Build fix in http://trac.webkit.org/changeset/175591