We provide web_view_get/set_editable() but HTML composers need more convenience API which we need to support.
The Evolution email client's composer is a good place to look for inspiration. The important things we can learn from it: gtkhtml_editor_run_command() ^ For running editor commands. We have this in WebKit but the command strings may be different. gtkhtml_editor_get_action() gtkhtml_editor_get_ui_manager() ^ For hooking up UI actions directly to the composer. gtkhtml_editor_get_html() gtkhtml_editor_set_html() ^ For accessing the full content. gtkhtml_editor_get_paragraph_data() gtkhtml_editor_set_paragraph_data() ^ For accessing individual paragraph contents(?) It will also be important to hook up Undo/Redo for editing, which is available in WebKit but not yet hooked up in the GTK+ port. Note the ability to group editing actions and label them in undo history. Other things to note are the spell checking support and smiley conversion (we should make it easy for this kind of cute feature to be implemented directly in the app since it doesn't really belong in the widget). Another interesting design decision is that the editor is a subclass of the HTML widget. It might be worth doing this kind of separation over WebView. See also: http://www.go-evolution.org/New_Composer Grep of gtkhtml use in the composer: gnome/evolution/composer$ grep gtkhtml_ * e-composer-actions.c: response = gtkhtml_editor_file_chooser_dialog_run ( e-composer-actions.c: if (!gtkhtml_editor_get_changed (editor) && e-composer-actions.c: gtkhtml_editor_set_changed (editor, TRUE); e-composer-actions.c: gtkhtml_editor_set_changed (editor, TRUE); e-composer-actions.c: filename = gtkhtml_editor_get_filename (editor); e-composer-actions.c: if (!gtkhtml_editor_save (editor, filename, TRUE, &error)) { e-composer-actions.c: gtkhtml_editor_run_command (GTKHTML_EDITOR (composer), "saved"); e-composer-actions.c: response = gtkhtml_editor_file_chooser_dialog_run ( e-composer-actions.c: gtkhtml_editor_set_filename (GTKHTML_EDITOR (composer), filename); e-composer-actions.c: gtkhtml_editor_set_changed (editor, TRUE); e-composer-actions.c: gtkhtml_editor_set_changed (editor, TRUE); e-composer-actions.c: manager = gtkhtml_editor_get_ui_manager (GTKHTML_EDITOR (composer)); e-composer-actions.h: (gtkhtml_editor_get_action (GTKHTML_EDITOR (composer), (name))) e-composer-autosave.c: if (!gtkhtml_editor_get_changed (editor)) e-composer-autosave.c: gtkhtml_editor_set_changed (editor, FALSE); e-composer-private.c: manager = gtkhtml_editor_get_ui_manager (GTKHTML_EDITOR (composer)); e-composer-private.c: manager = gtkhtml_editor_get_ui_manager (GTKHTML_EDITOR (composer)); e-composer-private.c: manager = gtkhtml_editor_get_ui_manager (editor); e-msg-composer.c: text = gtkhtml_editor_get_text_plain (editor, &length); e-msg-composer.c: gtkhtml_editor_run_command (editor, "save-data-on"); e-msg-composer.c: text = gtkhtml_editor_get_text_html (editor, &length); e-msg-composer.c: gtkhtml_editor_run_command (editor, "save-data-off"); e-msg-composer.c: gtkhtml_editor_set_text_html (GTKHTML_EDITOR (composer), body, -1); e-msg-composer.c: gtkhtml_editor_set_changed (editor, TRUE); e-msg-composer.c: gtkhtml_editor_set_inline_spelling (editor, enable); e-msg-composer.c: gtkhtml_editor_set_magic_links (editor, enable); e-msg-composer.c: gtkhtml_editor_set_magic_smileys (editor, enable); e-msg-composer.c: gtkhtml_editor_set_changed (editor, TRUE); e-msg-composer.c: gtkhtml_editor_set_html_mode (GTKHTML_EDITOR (composer), active); e-msg-composer.c: language = gtkhtml_spell_language_lookup (language_code); e-msg-composer.c: gtkhtml_editor_set_spell_languages ( e-msg-composer.c: gtkhtml_editor_run_command (GTKHTML_EDITOR (widget), "grab-focus"); e-msg-composer.c: gtkhtml_editor_run_command ( e-msg-composer.c: data = gtkhtml_editor_get_paragraph_data (editor, "orig"); e-msg-composer.c: gtkhtml_editor_run_command (editor, "text-default-color"); e-msg-composer.c: gtkhtml_editor_run_command (editor, "italic-off"); e-msg-composer.c: data = gtkhtml_editor_get_paragraph_data (editor, "signature"); e-msg-composer.c: gtkhtml_editor_run_command (editor, "text-default-color"); e-msg-composer.c: gtkhtml_editor_run_command (editor, "italic-off"); e-msg-composer.c: gtkhtml_editor_run_command (editor, "italic-off"); e-msg-composer.c: data = gtkhtml_editor_get_paragraph_data (editor, "orig"); e-msg-composer.c: gtkhtml_editor_set_paragraph_data (editor, "orig", "0"); e-msg-composer.c: data = gtkhtml_editor_get_paragraph_data (editor, "signature"); e-msg-composer.c: if (gtkhtml_editor_is_paragraph_empty (editor)) e-msg-composer.c: gtkhtml_editor_set_paragraph_data (editor, "signature" ,"0"); e-msg-composer.c: else if (gtkhtml_editor_is_previous_paragraph_empty (editor) && e-msg-composer.c: gtkhtml_editor_run_command (editor, "cursor-backward")) { e-msg-composer.c: gtkhtml_editor_set_paragraph_data (editor, "signature", "0"); e-msg-composer.c: gtkhtml_editor_run_command (editor, "cursor-forward"); e-msg-composer.c: gtkhtml_editor_run_command (editor, "text-default-color"); e-msg-composer.c: gtkhtml_editor_run_command (editor, "italic-off"); e-msg-composer.c: if (!gtkhtml_editor_is_paragraph_empty (editor)) e-msg-composer.c: data = gtkhtml_editor_get_paragraph_data (editor, "orig"); e-msg-composer.c: gtkhtml_editor_set_paragraph_data (editor, "orig", "0"); e-msg-composer.c: gtkhtml_editor_run_command (editor, "indent-zero"); e-msg-composer.c: gtkhtml_editor_run_command (editor, "style-normal"); e-msg-composer.c: gtkhtml_editor_run_command (editor, "text-default-color"); e-msg-composer.c: gtkhtml_editor_run_command (editor, "italic-off"); e-msg-composer.c: gtkhtml_editor_run_command (editor, "insert-paragraph"); e-msg-composer.c: gtkhtml_editor_run_command (editor, "delete-back"); e-msg-composer.c: data = gtkhtml_editor_get_paragraph_data (editor, "signature"); e-msg-composer.c: gtkhtml_editor_set_paragraph_data (editor, "signature", "0"); e-msg-composer.c: html = gtkhtml_editor_get_html (editor); e-msg-composer.c: gtkhtml_editor_get_html (editor), stream, e-msg-composer.c: html = gtkhtml_editor_get_html (editor); e-msg-composer.c: manager = gtkhtml_editor_get_ui_manager (editor); e-msg-composer.c: gtkhtml_editor_set_changed (editor, FALSE); e-msg-composer.c: if (!gtkhtml_editor_search_by_data (editor, 1, "ClueFlow", "signature", "1")) e-msg-composer.c: data = gtkhtml_editor_get_paragraph_data (editor, "signature_name"); e-msg-composer.c: gtkhtml_editor_set_html_mode ( e-msg-composer.c: gtkhtml_editor_set_html_mode ( e-msg-composer.c: gtkhtml_editor_run_command (editor, "editable-off"); e-msg-composer.c: gtkhtml_editor_set_changed (editor, FALSE); e-msg-composer.c: html_content = gtkhtml_editor_get_html_mode (editor); e-msg-composer.c: gtkhtml_editor_set_html_mode (GTKHTML_EDITOR (composer), FALSE); e-msg-composer.c: html_content = gtkhtml_editor_get_html_mode (editor); e-msg-composer.c: html_content = gtkhtml_editor_get_html_mode (editor); e-msg-composer.c: html_content = gtkhtml_editor_get_html_mode (editor); e-msg-composer.c: html = gtkhtml_editor_get_html (editor); e-msg-composer.c: gtkhtml_editor_freeze (editor); e-msg-composer.c: gtkhtml_editor_run_command (editor, "cursor-position-save"); e-msg-composer.c: gtkhtml_editor_undo_begin (editor, "Set signature", "Reset signature"); e-msg-composer.c: gtkhtml_editor_run_command (editor, "block-selection"); e-msg-composer.c: gtkhtml_editor_run_command (editor, "cursor-bod"); e-msg-composer.c: if (gtkhtml_editor_search_by_data (editor, 1, "ClueFlow", "signature", "1")) { e-msg-composer.c: gtkhtml_editor_run_command (editor, "select-paragraph"); e-msg-composer.c: gtkhtml_editor_run_command (editor, "delete"); e-msg-composer.c: gtkhtml_editor_set_paragraph_data (editor, "signature", "0"); e-msg-composer.c: gtkhtml_editor_run_command (editor, "delete-back"); e-msg-composer.c: gtkhtml_editor_run_command (editor, "unblock-selection"); e-msg-composer.c: gtkhtml_editor_run_command (editor, "insert-paragraph"); e-msg-composer.c: if (!gtkhtml_editor_run_command (editor, "cursor-backward")) e-msg-composer.c: gtkhtml_editor_run_command (editor, "insert-paragraph"); e-msg-composer.c: gtkhtml_editor_run_command (editor, "cursor-forward"); e-msg-composer.c: gtkhtml_editor_set_paragraph_data (editor, "orig", "0"); e-msg-composer.c: gtkhtml_editor_run_command (editor, "indent-zero"); e-msg-composer.c: gtkhtml_editor_run_command (editor, "style-normal"); e-msg-composer.c: gtkhtml_editor_insert_html (editor, html_text); e-msg-composer.c: gtkhtml_editor_undo_end (editor); e-msg-composer.c: gtkhtml_editor_run_command (editor, "cursor-position-restore"); e-msg-composer.c: gtkhtml_editor_thaw (editor); e-msg-composer.c: text = gtkhtml_editor_get_text_plain (editor, &length); e-msg-composer.c: gtkhtml_editor_set_html_mode (editor, !alt); e-msg-composer.c: if (!gtkhtml_editor_is_paragraph_empty (editor)) { e-msg-composer.c: if (gtkhtml_editor_is_previous_paragraph_empty (editor)) e-msg-composer.c: gtkhtml_editor_run_command (editor, "cursor-backward"); e-msg-composer.c: gtkhtml_editor_run_command (editor, "text-default-color"); e-msg-composer.c: gtkhtml_editor_run_command (editor, "italic-off"); e-msg-composer.c: gtkhtml_editor_run_command (editor, "insert-paragraph"); e-msg-composer.c: gtkhtml_editor_run_command (editor, "style-normal"); e-msg-composer.c: gtkhtml_editor_run_command (editor, "indent-zero"); e-msg-composer.c: gtkhtml_editor_run_command (editor, "text-default-color"); e-msg-composer.c: gtkhtml_editor_run_command (editor, "italic-off");
(In reply to comment #1) > gtkhtml_editor_get_paragraph_data() > gtkhtml_editor_set_paragraph_data() > > ^ For accessing individual paragraph contents(?) I think this is for tagging paragraphs with metadata, such as whether it's a signature paragraph or not. Evolution treats signature paragraphs special. > Other things to note are the spell checking support and smiley conversion (we > should make it easy for this kind of cute feature to be implemented directly > in the app since it doesn't really belong in the widget). If WebKit could not try to implement spell checking itself but just provide a suitable virtual API to subclass and the ability to mark misspellings, that would be preferable. > Another interesting design decision is that the editor is a subclass of the > HTML widget. It might be worth doing this kind of separation over WebView. To clarify, the HTML editor is just a GtkWindow and contains a GtkHTML instance. Evolution's message composer is a subclass of the HTML editor window. I anticipate swapping the GtkHTML instance for a WebView instance without changing the fundamental design. Also, does WebKit have the capacity to convert HTML content to plain text and vice versa? GtkHtml has such a converter but it may need some updating.
Created attachment 45136 [details] Implement webkit_web_view_execute_command and command-executed signal This adds a function to execute commands, such as to make text bold, move text and other things. I'm not sure what the best way to document it is. I guess the documentation should list commands... if you agree with the functionality I intend to look into documenting it. Use cases: Implement custom context menu Keep a history of executed commands Execute commands with custom key bindings Prevent commands to restrict text editing
Attachment 45136 [details] did not pass style-queue: Failed to run "WebKitTools/Scripts/check-webkit-style" exit_code: 1 WebKit/gtk/WebCoreSupport/EditorClientGtk.cpp:483: Extra space before ( in function call [whitespace/parens] [4] WebKit/gtk/webkit/webkitwebview.cpp:2117: Use 0 instead of NULL. [readability/null] [5] WebKit/gtk/webkit/webkitwebview.cpp:3941: webkit_web_view_execute_command is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] WebKit/gtk/webkit/webkitwebview.cpp:3948: Extra space before ( in function call [whitespace/parens] [4] WebKit/gtk/webkit/webkitwebview.h:360: Extra space before ( in function call [whitespace/parens] [4] Total errors found: 5
Created attachment 45143 [details] Implement webkit_web_view_execute_command, command-executed, catch internal commands I updated the patch since I noticed that commands coming from WebCore, ie. through Javascript were not handled at all. So I added a shouldExecuteCommand to EditorClient. One remaining issue: for some reason internal commands have no command names.
Attachment 45143 [details] did not pass style-queue: Failed to run "WebKitTools/Scripts/check-webkit-style" exit_code: 1 WebKit/gtk/WebCoreSupport/EditorClientGtk.cpp:109: Extra space before ( in function call [whitespace/parens] [4] WebKit/gtk/webkit/webkitwebview.cpp:2118: Use 0 instead of NULL. [readability/null] [5] WebKit/gtk/webkit/webkitwebview.cpp:3946: webkit_web_view_execute_command is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] WebKit/gtk/webkit/webkitwebview.h:360: Extra space before ( in function call [whitespace/parens] [4] Total errors found: 4
Comment on attachment 45143 [details] Implement webkit_web_view_execute_command, command-executed, catch internal commands > +#if PLATFORM(GTK) > + if (!m_frame->editor()->client()->shouldExecuteCommand(m_command->value (m_frame.get(), triggeringEvent))) > + return false; > +#endif 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; > + > +#if PLATFORM(GTK) > + virtual bool shouldExecuteCommand(const String&) = 0; > +#endif Same here. Just add the function, and make sure to add it to all the ports. > + /** > + * WebKitWebView::command-executed: 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. > + * Since: 1.1.14 1.1.19, I guess =) > + webkit_web_view_signals[RESOURCE_REQUEST_STARTING] = g_signal_new("command-executed", wrong index > +/** > + * webkit_web_view_execute_command: 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. > + * @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.
Created attachment 82624 [details] Implement webkit_web_view_execute_editor_command and editor-command-executed (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.
Attachment 82624 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/ChangeLog', u'Source/WebCor..." exit_code: 1 Source/WebKit/gtk/webkit/webkitwebview.h:387: Extra space before ( in function call [whitespace/parens] [4] Source/WebKit/gtk/webkit/webkitwebview.h:387: WEBKIT_API should only appear in the chromium public directory. [readability/webkit_api] [5] Source/WebKit/gtk/webkit/webkitwebview.h:387: The parameter name "web_view" adds no information, so it should be removed. [readability/parameter_name] [5] Source/WebKit/gtk/WebCoreSupport/EditorClientGtk.cpp:275: Extra space before ( in function call [whitespace/parens] [4] Total errors found: 4 in 10 files If any of these errors are false positives, please file a bug against check-webkit-style.
Attachment 82624 [details] did not build on chromium: Build output: http://queues.webkit.org/results/7917350
Created attachment 82625 [details] Implement webkit_web_view_execute_editor_command and editor-command-executed My apologies, the previous patch was not quite uptodate.
Attachment 82625 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/ChangeLog', u'Source/WebCor..." exit_code: 1 Tools/GtkLauncher/main.c:238: Extra space before ( in function call [whitespace/parens] [4] Tools/GtkLauncher/main.c:242: Extra space before ( in function call [whitespace/parens] [4] Source/WebKit/gtk/webkit/webkitwebview.h:387: Extra space before ( in function call [whitespace/parens] [4] Source/WebKit/gtk/webkit/webkitwebview.h:387: WEBKIT_API should only appear in the chromium public directory. [readability/webkit_api] [5] Source/WebKit/gtk/webkit/webkitwebview.h:387: The parameter name "web_view" adds no information, so it should be removed. [readability/parameter_name] [5] Source/WebKit/gtk/WebCoreSupport/EditorClientGtk.cpp:275: Extra space before ( in function call [whitespace/parens] [4] Total errors found: 6 in 11 files If any of these errors are false positives, please file a bug against check-webkit-style.
Attachment 82624 [details] did not build on qt: Build output: http://queues.webkit.org/results/7926222
Attachment 82625 [details] did not build on chromium: Build output: http://queues.webkit.org/results/7920279
Attachment 82625 [details] did not build on qt: Build output: http://queues.webkit.org/results/7927169
Attachment 82625 [details] did not build on win: Build output: http://queues.webkit.org/results/7918303
Attachment 82625 [details] did not build on chromium: Build output: http://queues.webkit.org/results/7927190
Attachment 82625 [details] did not build on mac: Build output: http://queues.webkit.org/results/7927207
Comment on attachment 82625 [details] Implement webkit_web_view_execute_editor_command and editor-command-executed r- for the build breaks and style issues. Do you mind checking if the editing delegate signals added not so long ago fulfill your requirements instead of adding a new signal?
(In reply to comment #19) > (From update of attachment 82625 [details]) > r- for the build breaks and style issues. Do you mind checking if the editing delegate signals added not so long ago fulfill your requirements instead of adding a new signal? "should-begin-editing" "should-end-editing" "should-insert-node" "should-insert-text" "should-delete-range" "should-change-selected-range" "should-apply-style" "user-changed-contents" "editing-ended" "selection-changed" All of them are key binding/ action signals, first wins. That means I can't intercept changes individually. And as far as I see, I can't see what changed, it only tells me that something changed. Please correct me if I'm wrong here. Let's take an example: I want to prevent users from making text bold or italic. Control+B and Control+I allow that even if I don't provide any buttons. So I need a signal to catch the style change and prevent it. I still want to allow normal editing commands of course, so I can't use a first_wins signal. I also don't want users to affect text I previously styled as a quotation, so I need to prevent colour changes. To be exact, cursor information would be useful for that which my proposed signal doesn't solve yet. I would appreciate thoughts on solving these use cases.
We have the ability to execute commands now (WebKit2, at least) and we have DOM bindings for complicated things. Perhaps we can reopen this when we have a better understanding of a usecase we don't support.