[webkit-reviews] review requested: [Bug 19456] [GTK] Editing capabilities : [Attachment 82624] Implement webkit_web_view_execute_editor_command and editor-command-executed

[bugzilla-daemon at webkit.org]

Christian Dywan <christian at twotoasts.de> has asked  for review:
Bug 19456: [GTK] Editing capabilities
https://bugs.webkit.org/show_bug.cgi?id=19456

Attachment 82624: Implement webkit_web_view_execute_editor_command and
editor-command-executed
https://bugs.webkit.org/attachment.cgi?id=82624&action=review

------- Additional Comments from Christian Dywan <christian at twotoasts.de>
(In reply to comment #7)
> Indentation seems off. I think we should not make this gtk-specific. Just add
a call to shouldExecuteCommand, and make the default implementation for all
ports except ours be return true;

I removed the platform guards now.

> Perhaps we should name this editor-command, and get away with the 'executed'
part? The command has not really been executed yet - it will be.

I renamed the signal "editor-command" and documented that it is called *before*
execution.

> > +	 webkit_web_view_signals[RESOURCE_REQUEST_STARTING] =
g_signal_new("command-executed",

Fixed.

> I don't like 'execute_command'. It is too generic, and it's not really easy
to figure out what we are talking about. I would prefer
webkit_web_view_execute_editor_command, or editing_command.

Renamed the function to execute_editor_command() taking two string arguments.
The second string argument depends on the command, for example an URL for
insertImage, createLink.

> > + * @web_view: a #WebKitWebView
> > + * @command: a command string
> > + *
> > + * Executes the specified command, if possible.
> 
> What makes it not possible? We should document that the command will be
executed in the focused frame.

I gave insertImage and ToggleBold as examples of failure. It fails if there is
no selection, because that is what the change applies to.

I listed useful commands there, the list is not complete and mostly based on
most likely useful commands.