[Webkit-unassigned] [Bug 138284] Implement action menus for editable text

[bugzilla-daemon at webkit.org]

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

--- Comment #4 from Beth Dakin <bdakin at apple.com> ---
(In reply to comment #3)
> Comment on attachment 240797 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=240797&action=review
> 
> > Source/WebKit2/UIProcess/mac/WKActionMenuController.mm:339
> > +-(void)_paste:(id)sender
> 
> "copyText", but "paste"? copy can copy things that aren't text; can we go
> with _copy and _paste or _performCopy and _performPaste or something?
> 

They are inconsistent. I do think that copy needs a more specific name because right now it will just copy the current selection. So perhaps copySelection is right for copy. paste still seems like the most accurate way to describe paste, but I suppose we could go with the somewhat redundant pasteFromPasteboard?! I'll think about it, but fix copy for sure.

> > Source/WebKit2/UIProcess/mac/WKActionMenuController.mm:496
> > +    return hitTestResult;
> 
> .release() here, I think
> 

Ah yes.

> > Source/WebKit2/UIProcess/mac/WKActionMenuController.mm:503
> > +        _type = kWKActionMenuNone;
> 
> unfortunate that the 'none' case is duplicated now.
> 

I know. It seems so crazy to have all the rest of it nested in an if though.

> > Source/WebKit2/UIProcess/mac/WKActionMenuController.mm:518
> > +        if (DDActionContext *actionContext = _hitTestResult.actionContext.get()) {
> 
> We should shuffle this out into a "defaultMenuItemsForDataDetectedItem" or
> something, so that these all look the same.

Will do.

Thanks you!

-- 
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/20141103/92ceefa2/attachment-0002.html>