From 5b8c68f5fa19b984959de1cd378f8caa6f7385b0 Mon Sep 17 00:00:00 2001 From: Michael James Gratton Date: Thu, 1 Dec 2016 12:06:44 +1100 Subject: [PATCH] Implement getting message selection for quoting and selection in WK2. * src/client/conversation-viewer/conversation-web-view.vala (ConversationWebView): Remove has_selection method since we are using the signal to specify if a selection was found or not. Update call sites to use that. (ConversationWebView::get_selection_for_find, ConversationWebView::get_selection_for_quoting): Implement using calls to web process JS methods. * src/client/application/geary-controller.vala (GearyController::create_reply_forward_widget): If we have a possible message view to quote from, handle constructing the compser widget asynchronously, when we know if we have a quote or not. * src/client/conversation-viewer/conversation-viewer.vala: (ConversationViewer::on_find_mode_changed): Handle getting text selection for finds asynchonously. * src/client/components/client-web-view.vala (ClientWebView::get_string_result): New helper for getting string values from JS calls. * src/client/conversation-viewer/conversation-email.vala (ConversationEmail::get_selection_for_quoting, ConversationEmail::get_selection_for_find): Handxle errors when obtaining selections from a message view. * src/client/conversation-viewer/conversation-message.vala: Remove methods that were simply passed through to the web view anyway. Update call sies. * src/client/web-process/util-conversation.vala: Port all remaining functions to JS, remove. * bindings/vapi/javascriptcore-4.0.vapi: Add methods needed to get strings out of WebKit.JavascriptResult instances. * ui/conversation-web-view.js: Implement selection functions in JS, minor cleanup. --- bindings/vapi/javascriptcore-4.0.vapi | 20 ++++- src/CMakeLists.txt | 1 - src/client/application/geary-controller.vala | 10 ++- src/client/components/client-web-view.vala | 17 ++++ .../conversation-email.vala | 41 ++++++---- .../conversation-message.vala | 11 --- .../conversation-viewer.vala | 12 +-- .../conversation-web-view.vala | 15 ++-- src/client/web-process/util-conversation.vala | 78 ------------------- ui/conversation-web-view.js | 59 +++++++++++++- 10 files changed, 137 insertions(+), 127 deletions(-) delete mode 100644 src/client/web-process/util-conversation.vala diff --git a/bindings/vapi/javascriptcore-4.0.vapi b/bindings/vapi/javascriptcore-4.0.vapi index 027d781b..f17c5d19 100644 --- a/bindings/vapi/javascriptcore-4.0.vapi +++ b/bindings/vapi/javascriptcore-4.0.vapi @@ -19,6 +19,9 @@ namespace JS { [CCode (cname = "JSValueToNumber")] public double to_number(JS.Value value, out JS.Value exception); + [CCode (cname = "JSValueToStringCopy")] + public String to_string_copy(JS.Value value, out JS.Value exception); + [CCode (cname = "JSGlobalContextRelease")] public bool release(); } @@ -68,6 +71,21 @@ namespace JS { [CCode (cname = "JSStringCreateWithUTF8CString")] public String.create_with_utf8_cstring(string str); - + + [CCode (cname = "JSStringGetLength")] + public int String.get_length(); + + [CCode (cname = "JSStringGetMaximumUTF8CStringSize")] + public int String.get_maximum_utf8_cstring_size(); + + [CCode (cname = "JSStringGetUTF8CString")] + public void String.get_utf8_cstring(string* buffer, int bufferSize); + + [CCode (cname = "JSStringRetain")] + public void String.retain(); + + [CCode (cname = "JSStringRelease")] + public void String.release(); + } } diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index 46c23b39..40651c1d 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -408,7 +408,6 @@ client/util/util-migrate.vala set(WEB_PROCESS_SRC client/web-process/web-process-extension.vala client/web-process/util-composer.vala -client/web-process/util-conversation.vala client/web-process/util-webkit.vala ) diff --git a/src/client/application/geary-controller.vala b/src/client/application/geary-controller.vala index 0c7148bc..d221c47e 100644 --- a/src/client/application/geary-controller.vala +++ b/src/client/application/geary-controller.vala @@ -2278,11 +2278,15 @@ public class GearyController : Geary.BaseObject { email_view = list_view.get_reply_target(); } } - string? quote = null; + if (email_view != null) { - quote = email_view.get_selection_for_quoting(); + email_view.get_selection_for_quoting.begin((obj, res) => { + string? quote = email_view.get_selection_for_quoting.end(res); + create_compose_widget(compose_type, email_view.email, quote); + }); + } else { + create_compose_widget(compose_type, email_view.email, null); } - create_compose_widget(compose_type, email_view.email, quote); } private void create_compose_widget(ComposerWidget.ComposeType compose_type, diff --git a/src/client/components/client-web-view.vala b/src/client/components/client-web-view.vala index 2a49b8e8..ff05cc78 100644 --- a/src/client/components/client-web-view.vala +++ b/src/client/components/client-web-view.vala @@ -100,6 +100,23 @@ public class ClientWebView : WebKit.WebView { } JS.Value? err = null; return (int) context.to_number(value, out err); + // XXX check err + // XXX unref result? + } + + protected static string get_string_result(WebKit.JavascriptResult result) + throws JSError { + JS.GlobalContext context = result.get_global_context(); + JS.Value js_str_value = result.get_value(); + JS.Value? err = null; + JS.String js_str = context.to_string_copy(js_str_value, out err); + // XXX check err + int len = js_str.get_maximum_utf8_cstring_size(); + string value = string.nfill(len, 0); + js_str.get_utf8_cstring(value, len); + js_str.release(); + debug("Got string: %s", value); + return value; // XXX unref result? } diff --git a/src/client/conversation-viewer/conversation-email.vala b/src/client/conversation-viewer/conversation-email.vala index 30bc0bc9..b1dafd7c 100644 --- a/src/client/conversation-viewer/conversation-email.vala +++ b/src/client/conversation-viewer/conversation-email.vala @@ -575,19 +575,33 @@ public class ConversationEmail : Gtk.Box { /** * Returns user-selected body HTML from a message, if any. */ - public string? get_selection_for_quoting() { - return (this.body_selection_message != null) - ? this.body_selection_message.get_selection_for_quoting() - : null; + public async string? get_selection_for_quoting() { + string? selection = null; + if (this.body_selection_message != null) { + try { + selection = + yield this.body_selection_message.web_view.get_selection_for_quoting(); + } catch (Error err) { + debug("Failed to get selection for quoting: %s", err.message); + } + } + return selection; } /** * Returns user-selected body text from a message, if any. */ - public string? get_selection_for_find() { - return (this.body_selection_message != null) - ? this.body_selection_message.get_selection_for_find() - : null; + public async string? get_selection_for_find() { + string? selection = null; + if (this.body_selection_message != null) { + try { + selection = + yield this.body_selection_message.web_view.get_selection_for_find(); + } catch (Error err) { + debug("Failed to get selection for find: %s", err.message); + } + } + return selection; } /** @@ -630,8 +644,9 @@ public class ConversationEmail : Gtk.Box { this.message_bodies_loaded = true; } }); - view.web_view.selection_changed.connect(() => { - on_message_selection_changed(view); + view.web_view.selection_changed.connect((has_selection) => { + this.body_selection_message = has_selection ? view : null; + body_selection_changed(has_selection); }); } @@ -766,12 +781,6 @@ public class ConversationEmail : Gtk.Box { contact_store.mark_contacts_async.begin(contact_list, flags, null); } - private void on_message_selection_changed(ConversationMessage view) { - bool has_selection = view.web_view.has_selection(); - this.body_selection_message = has_selection ? view : null; - body_selection_changed(has_selection); - } - [GtkCallback] private void on_attachments_child_activated(Gtk.FlowBox view, Gtk.FlowBoxChild child) { diff --git a/src/client/conversation-viewer/conversation-message.vala b/src/client/conversation-viewer/conversation-message.vala index 7c567d4b..0b3e65fb 100644 --- a/src/client/conversation-viewer/conversation-message.vala +++ b/src/client/conversation-viewer/conversation-message.vala @@ -514,17 +514,6 @@ public class ConversationMessage : Gtk.Grid { web_view.get_find_controller().search_finish(); } - internal string? get_selection_for_quoting() { - return this.web_view.get_selection_for_quoting(); - } - - /** - * Returns the current selection as a string, suitable for find. - */ - internal string? get_selection_for_find() { - return this.web_view.get_selection_for_find(); - } - private SimpleAction add_action(string name, bool enabled, VariantType? type = null) { SimpleAction action = new SimpleAction(name, type); action.set_enabled(enabled); diff --git a/src/client/conversation-viewer/conversation-viewer.vala b/src/client/conversation-viewer/conversation-viewer.vala index 7ad8c45e..aba2442e 100644 --- a/src/client/conversation-viewer/conversation-viewer.vala +++ b/src/client/conversation-viewer/conversation-viewer.vala @@ -294,11 +294,13 @@ public class ConversationViewer : Gtk.Stack { ConversationEmail? email_view = this.current_list.get_selection_view(); if (email_view != null) { - string text = email_view.get_selection_for_find(); - if (text != null) { - this.conversation_find_entry.set_text(text); - this.conversation_find_entry.select_region(0, -1); - } + email_view.get_selection_for_find.begin((obj, res) => { + string text = email_view.get_selection_for_find.end(res); + if (text != null) { + this.conversation_find_entry.set_text(text); + this.conversation_find_entry.select_region(0, -1); + } + }); } } else { // Find was disabled diff --git a/src/client/conversation-viewer/conversation-web-view.vala b/src/client/conversation-viewer/conversation-web-view.vala index bbfc9514..0a84c18f 100644 --- a/src/client/conversation-viewer/conversation-web-view.vala +++ b/src/client/conversation-viewer/conversation-web-view.vala @@ -39,23 +39,20 @@ public class ConversationWebView : ClientWebView { load_html(html, null); } - public bool has_selection() { - bool has_selection = false; // XXX set me - return has_selection; - } - /** * Returns the current selection, for prefill as find text. */ - public string get_selection_for_find() { - return ""; // XXX + public async string get_selection_for_find() throws Error{ + WebKit.JavascriptResult result = yield this.run_javascript("geary.getSelectionForFind();", null); + return get_string_result(result); } /** * Returns the current selection, for quoting in a message. */ - public string get_selection_for_quoting() { - return ""; // XXX + public async string get_selection_for_quoting() throws Error { + WebKit.JavascriptResult result = yield this.run_javascript("geary.getSelectionForQuoting();", null); + return get_string_result(result); } /** diff --git a/src/client/web-process/util-conversation.vala b/src/client/web-process/util-conversation.vala deleted file mode 100644 index 04c95e83..00000000 --- a/src/client/web-process/util-conversation.vala +++ /dev/null @@ -1,78 +0,0 @@ -/* - * Copyright 2016 Software Freedom Conservancy Inc. - * Copyright 2016 Michael Gratton - * - * This software is licensed under the GNU Lesser General Public License - * (version 2.1 or later). See the COPYING file in this distribution. - */ - -namespace Util.Conversation { - - private const string QUOTE_CONTAINER_CLASS = "geary_quote_container"; - private const string QUOTE_CONTROLLABLE_CLASS = "controllable"; - private const string QUOTE_HIDE_CLASS = "hide"; - private const float QUOTE_SIZE_THRESHOLD = 2.0f; - - - public string? get_selection_for_quoting(WebKit.WebPage page) { - string? quote = null; - // WebKit.DOM.Document document = page.get_dom_document(); - // WebKit.DOM.DOMSelection selection = document.default_view.get_selection(); - // if (!selection.is_collapsed) { - // try { - // WebKit.DOM.Range range = selection.get_range_at(0); - // WebKit.DOM.HTMLElement dummy = - // (WebKit.DOM.HTMLElement) document.create_element("div"); - // bool include_dummy = false; - // WebKit.DOM.Node ancestor_node = range.get_common_ancestor_container(); - // WebKit.DOM.Element? ancestor = ancestor_node as WebKit.DOM.Element; - // if (ancestor == null) - // ancestor = ancestor_node.get_parent_element(); - // // If the selection is part of a plain text message, - // // we have to stick it in an appropriately styled div, - // // so that new lines are preserved. - // if (Util.DOM.is_descendant_of(ancestor, ".plaintext")) { - // dummy.get_class_list().add("plaintext"); - // dummy.set_attribute("style", "white-space: pre-wrap;"); - // include_dummy = true; - // } - // dummy.append_child(range.clone_contents()); - - // // Remove the chrome we put around quotes, leaving - // // only the blockquote element. - // WebKit.DOM.NodeList quotes = - // dummy.query_selector_all("." + QUOTE_CONTAINER_CLASS); - // for (int i = 0; i < quotes.length; i++) { - // WebKit.DOM.Element div = (WebKit.DOM.Element) quotes.item(i); - // WebKit.DOM.Element blockquote = div.query_selector("blockquote"); - // div.get_parent_element().replace_child(blockquote, div); - // } - - // quote = include_dummy ? dummy.get_outer_html() : dummy.get_inner_html(); - // } catch (Error error) { - // debug("Problem getting selected text: %s", error.message); - // } - // } - return quote; - } - - public string? get_selection_for_find(WebKit.WebPage page) { - string? value = null; - // WebKit.DOM.Document document = page.get_dom_document(); - // WebKit.DOM.DOMWindow window = document.get_default_view(); - // WebKit.DOM.DOMSelection selection = window.get_selection(); - - // if (selection.get_range_count() > 0) { - // try { - // WebKit.DOM.Range range = selection.get_range_at(0); - // value = range.get_text().strip(); - // if (value.length <= 0) - // value = null; - // } catch (Error e) { - // warning("Could not get selected text from web view: %s", e.message); - // } - // } - return value; - } - -} diff --git a/ui/conversation-web-view.js b/ui/conversation-web-view.js index 15db7fbc..38e70af8 100644 --- a/ui/conversation-web-view.js +++ b/ui/conversation-web-view.js @@ -1,4 +1,5 @@ /* + * Copyright 2016 Software Freedom Conservancy Inc. * Copyright 2016 Michael Gratton * * This software is licensed under the GNU Lesser General Public License @@ -20,8 +21,8 @@ ConversationPageState.prototype = { this.updateDirection(); this.createControllableQuotes(); this.wrapSignature(); - // Call after so we continue to a preferred size update after - // munging the HTML above. + // Chain up here so we continue to a preferred size update + // after munging the HTML above. PageState.prototype.loaded.apply(this, []); }, /** @@ -49,7 +50,9 @@ ConversationPageState.prototype = { // top level blockquote if (!ConversationPageState.isDescendantOf(blockquote, "BLOCKQUOTE")) { var quoteContainer = document.createElement("DIV"); - quoteContainer.classList.add("geary-quote-container"); + quoteContainer.classList.add( + ConversationPageState.QUOTE_CONTAINER_CLASS + ); // Only make it controllable if the quote is tall enough if (blockquote.offsetHeight > 50) { @@ -116,9 +119,59 @@ ConversationPageState.prototype = { } while (elem != null); parent.appendChild(signatureContainer); } + }, + getSelectionForQuoting: function() { + var quote = null; + var selection = window.getSelection(); + if (!selection.isCollapsed) { + var range = selection.getRangeAt(0); + var dummy = document.createElement("DIV"); + var includeDummy = false; + var ancestor = range.commonAncestorContainer; + if (ancestor.nodeType != Node.ELEMENT_NODE) { + ancestor = ancestor.parentNode; + // If the selection is part of a plain text message, + // we have to stick it in an appropriately styled div, + // so that new lines are preserved. + if (ConversationPageState.isDescendantOf(ancestor, ".plaintext")) { + dummy.classList.add("plaintext"); + dummy.setAttribute("style", "white-space: pre-wrap;"); + includeDummy = true; + } + dummy.appendChild(range.cloneContents()); + + // Remove the chrome we put around quotes, leaving + // only the blockquote element. + var quotes = dummy.querySelectorAll( + "." + ConversationPageState.QUOTE_CONTAINER_CLASS + ); + for (var i = 0; i < quotes.length; i++) { + var div = quotes.item(i); + var blockquote = div.querySelector("blockquote"); + div.parentElement().replaceChild(blockquote, div); + } + + quote = includeDummy ? dummy.outerHTML : dummy.innerHTML; + } + } + return quote; + }, + getSelectionForFind: function() { + var value = null; + var selection = window.getSelection(); + + if (selection.rangeCount > 0) { + var range = selection.getRangeAt(0); + value = range.toString().trim(); + if (value == "") { + value = null; + } + } + return value; } }; +ConversationPageState.QUOTE_CONTAINER_CLASS = "geary-quote-container"; ConversationPageState.isDescendantOf = function(node, ancestorTag) { var ancestor = node.parentNode; while (ancestor != null) {