[Webkit-unassigned] [Bug 138432] Action menu URL preview should "peek, " i.e. appear when the menu item is highlighted

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=138432

--- Comment #3 from Beth Dakin <bdakin at apple.com> ---
(In reply to comment #2)
> Comment on attachment 241046 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=241046&action=review
> 
> > Source/WebKit2/UIProcess/mac/WKActionMenuController.h:59
> > +    RetainPtr<NSPopover> _previewPopover;
> 
> I wonder if we should close the popover and nil out its delegate when the
> action menu controller is deallocated just to be on the safe side.
> 

Sounds like a good idea. I'll put it in willDestroyView for now, but maybe we should have a dealloc too?

> > Source/WebKit2/UIProcess/mac/WKActionMenuController.mm:180
> > +    if (_previewPopover && !_shouldKeepPreviewPopoverOpen) {
> 
> The _previewPopover check is unnecessary.
> 

Removed.

> > Source/WebKit2/UIProcess/mac/WKActionMenuController.mm:182
> > +        _previewPopover = nil;
> 
> It would probably be a good idea to nil out the popover's delegate as part
> of tear-down.
> 

Will do.

> > Source/WebKit2/UIProcess/mac/WKActionMenuController.mm:258
> > +- (void)_keepPreviewOpen:(id)sender
> 
> Should this be -_keepPreviewOpenFromActionMenu: for consistency?
> 

It should. Fixed.

> > Source/WebKit2/UIProcess/mac/WKActionMenuController.mm:267
> > +        return;
> 
> Is there any way for this to be received with a different URL from the one
> being shown? If there is, I'm not sure this will do the right thing.

Good point. This should not be possible… I'll give some thought to whether it deserves some sort of belt-and-suspenders.

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-unassigned/attachments/20141105/4ab62e43/attachment-0002.html>