Bug 49829 – WebKit2: Add WKViewRef API for executing edit commands

• Component: WebKit2
• Status: RESOLVED FIXED

[Sam Weinig]

We need API for SelectAll, Undo and Redo on WKView. In WebKit1, this was handled by a custom WM_COMMAND, but there is no reason to copy that approach.

[Sam Weinig]

<rdar://problem/8705950>

[Jeff Miller]

*** Bug 55710 has been marked as a duplicate of this bug. ***

[Jeff Miller]

I believe the commands we need to support are cut/copy/paste, delete, select all, and undo/redo.

[Jeff Miller]

Created attachment 84632 [details]
Patch

[Darin Adler]

Comment on attachment 84632 [details]
Patch

I think this should be a WKPage API, not a WKView API. I also think this should take WebCore command names rather than a fixed set of command numbers.

I’m going to say review- for now, but you can put this back up for review if others disagree.

[Jeff Miller]

Because this is a Windows-specific API, both Sam and Anders agree it should be in WKView, since WKPage has no Windows-specific implementation.  I would rather use the WebCore Editor command name strings directly, but I'm not sure what the best way to expose these through the API (if we need to do that at all, we could just assume the client knows what the command names are, I suppose).

[Jeff Miller]

I suggested to Anders that we could just document the Editor command names in a comment block in WKView.h, and he agreed.  I'll submit another patch that uses the command names instead, but I'm going to leave the API in WKView.

[Jeff Miller]

Created attachment 84645 [details]
Patch

[Anders Carlsson]

Comment on attachment 84645 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=84645&action=review

> Source/WebKit2/UIProcess/API/C/win/WKView.h:54
> +    Valid command name strings include:

Maybe this could mention that it accepts all WebCore editing commands?

> Source/WebKit2/UIProcess/API/C/win/WKView.h:64
> +WK_EXPORT void WKViewExecuteCommand(WKViewRef view, WKStringRef command);

It's probably simpler if this takes a const char* since edit commands are all ASCII and that would make it easier for API clients.

[Anders Carlsson]

Comment on attachment 84645 [details]
Patch

Nah, it's fine to take a WKStringRef. I changed my mind.

[Jeff Miller]

Committed r80295: <http://trac.webkit.org/changeset/80295>

[Darin Adler]

(In reply to comment #6)
> Because this is a Windows-specific API

Why is this a Windows-specific API?

[Darin Adler]

I’d love to have this API on all platforms.

[Jeff Miller]

(In reply to comment #13)
> I’d love to have this API on all platforms.

There's a semantic mismatch between Mac and Windows in where menu items are set up (and executed).

On the Mac, we can do this all in the OpenSource side of the world in WKView.mm. On Windows, though, we have to do all this in the app.

According to Sam, we're not defining the final WebKit2 API set at this point, we're focused on making the API work for Safari specifically in the short term.  We can certainly revisit this when we are ready to think about an API design that will work for more clients.

[Darin Adler]

I still think this call should have gone on WKPage. I don’t see any argument here for why putting it on WKView is superior. The argument seems to be “there is code a little like this in the Mac’s WKView” and “we haven’t needed this on Mac” and neither convinces me. I’m slightly annoyed that we added this to what seems to me to be the wrong class.

[Jeff Miller]

OK, I will move this API to WKPage.  New bug coming soon.

[Jeff Miller]

Created bug 55744.