From d7af23201c3b6a8e4e9bf4887ddb3df7d46e5de4 Mon Sep 17 00:00:00 2001 From: Michael James Gratton Date: Tue, 25 Aug 2020 22:10:28 +1000 Subject: [PATCH 01/18] Revert "Revert "Merge branch 'mjog/558-webkit-shared-process' into 'mainline'"" This reverts commit cbe6e0ba9bc7d83a8a6c3eb2372f1c8fefd735b4, which reinstates commit e4a5b85698835549d823d3f501d398b411241a37. See !411 and !374 --- desktop/org.gnome.Geary.appdata.xml.in.in | 8 + po/POTFILES.in | 2 +- .../accounts/accounts-editor-edit-pane.vala | 4 +- .../accounts/accounts-signature-web-view.vala | 4 +- .../application/application-controller.vala | 5 +- src/client/application/main.vala | 6 - ...web-view.vala => components-web-view.vala} | 160 ++++++------- src/client/composer/composer-web-view.vala | 6 +- src/client/composer/composer-widget.vala | 8 +- .../conversation-email.vala | 26 +-- .../conversation-list-box.vala | 14 +- .../conversation-message.vala | 213 ++++++++++++++---- .../conversation-viewer.vala | 24 +- .../conversation-web-view.vala | 41 +++- src/client/meson.build | 12 +- .../web-process/web-process-extension.vala | 43 ++-- ...ala => components-web-view-test-case.vala} | 23 +- ...est.vala => components-web-view-test.vala} | 10 +- .../composer/composer-web-view-test.vala | 2 +- ...t.vala => components-page-state-test.vala} | 16 +- test/js/composer-page-state-test.vala | 2 +- test/js/conversation-page-state-test.vala | 2 +- test/meson.build | 6 +- test/test-client.vala | 6 +- ui/client-web-view-allow-remote-images.js | 11 - ...ent-web-view.js => components-web-view.js} | 5 +- ui/conversation-message.ui | 1 + ui/org.gnome.Geary.gresource.xml | 3 +- 28 files changed, 418 insertions(+), 245 deletions(-) rename src/client/components/{client-web-view.vala => components-web-view.vala} (92%) rename test/client/components/{client-web-view-test-case.vala => components-web-view-test-case.vala} (74%) rename test/client/components/{client-web-view-test.vala => components-web-view-test.vala} (79%) rename test/js/{client-page-state-test.vala => components-page-state-test.vala} (73%) delete mode 100644 ui/client-web-view-allow-remote-images.js rename ui/{client-web-view.js => components-web-view.js} (98%) diff --git a/desktop/org.gnome.Geary.appdata.xml.in.in b/desktop/org.gnome.Geary.appdata.xml.in.in index 7c1ddcab..471b69c4 100644 --- a/desktop/org.gnome.Geary.appdata.xml.in.in +++ b/desktop/org.gnome.Geary.appdata.xml.in.in @@ -88,6 +88,14 @@ geary + + +

Enhancements included in this release:

+
    +
  • Conversation loading performance improvements
    • +
+ +

Enhancements included in this release:

diff --git a/po/POTFILES.in b/po/POTFILES.in index 68e3ca34..cd8b339d 100644 --- a/po/POTFILES.in +++ b/po/POTFILES.in @@ -37,7 +37,6 @@ src/client/application/goa-mediator.vala src/client/application/main.vala src/client/application/secret-mediator.vala src/client/client-action.vala -src/client/components/client-web-view.vala src/client/components/components-attachment-pane.vala src/client/components/components-conversation-actions.vala src/client/components/components-conversation-action-bar.vala @@ -52,6 +51,7 @@ src/client/components/components-problem-report-info-bar.vala src/client/components/components-reflow-box.c src/client/components/components-search-bar.vala src/client/components/components-validator.vala +src/client/components/components-web-view.vala src/client/components/count-badge.vala src/client/components/folder-popover.vala src/client/components/icon-factory.vala diff --git a/src/client/accounts/accounts-editor-edit-pane.vala b/src/client/accounts/accounts-editor-edit-pane.vala index c1df9a95..2722db6e 100644 --- a/src/client/accounts/accounts-editor-edit-pane.vala +++ b/src/client/accounts/accounts-editor-edit-pane.vala @@ -718,7 +718,7 @@ internal class Accounts.RemoveMailboxCommand : Application.Command { internal class Accounts.SignatureChangedCommand : Application.Command { - private ClientWebView signature_view; + private Components.WebView signature_view; private Geary.AccountInformation account; private string old_value; @@ -728,7 +728,7 @@ internal class Accounts.SignatureChangedCommand : Application.Command { private bool new_enabled = false; - public SignatureChangedCommand(ClientWebView signature_view, + public SignatureChangedCommand(Components.WebView signature_view, Geary.AccountInformation account) { this.signature_view = signature_view; this.account = account; diff --git a/src/client/accounts/accounts-signature-web-view.vala b/src/client/accounts/accounts-signature-web-view.vala index ca31ad5e..d424dd64 100644 --- a/src/client/accounts/accounts-signature-web-view.vala +++ b/src/client/accounts/accounts-signature-web-view.vala @@ -8,14 +8,14 @@ /** * A class for editing signatures in the accounts editor. */ -public class Accounts.SignatureWebView : ClientWebView { +public class Accounts.SignatureWebView : Components.WebView { private static WebKit.UserScript? app_script = null; public static new void load_resources() throws GLib.Error { - SignatureWebView.app_script = ClientWebView.load_app_script( + SignatureWebView.app_script = Components.WebView.load_app_script( "signature-web-view.js" ); } diff --git a/src/client/application/application-controller.vala b/src/client/application/application-controller.vala index 234c7485..55864cda 100644 --- a/src/client/application/application-controller.vala +++ b/src/client/application/application-controller.vala @@ -134,13 +134,12 @@ internal class Application.Controller : this.upgrade_dialog = new UpgradeDialog(application); // Initialise WebKit and WebViews - ClientWebView.init_web_context( + Components.WebView.init_web_context( this.application.config, this.application.get_web_extensions_dir(), this.application.get_user_cache_directory().get_child("web-resources") ); - - ClientWebView.load_resources( + Components.WebView.load_resources( this.application.get_user_config_directory() ); Composer.WebView.load_resources(); diff --git a/src/client/application/main.vala b/src/client/application/main.vala index 741c0212..06b155dd 100644 --- a/src/client/application/main.vala +++ b/src/client/application/main.vala @@ -5,12 +5,6 @@ */ int main(string[] args) { - // Temporary workaround for WebKitGTK deprecation of the - // shared-secondary process model. Pull this out in 3.36 when the - // proper fix lands. See GNOME/geary#558. - Environment.set_variable("WEBKIT_USE_SINGLE_WEB_PROCESS", "1", true); - - // Init logging right up front so as to capture as many log // messages as possible Geary.Logging.init(); diff --git a/src/client/components/client-web-view.vala b/src/client/components/components-web-view.vala similarity index 92% rename from src/client/components/client-web-view.vala rename to src/client/components/components-web-view.vala index b65f2150..4bda1c11 100644 --- a/src/client/components/client-web-view.vala +++ b/src/client/components/components-web-view.vala @@ -1,6 +1,6 @@ /* * Copyright 2016 Software Freedom Conservancy Inc. - * Copyright 2016 Michael Gratton + * Copyright 2016-2019 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. @@ -14,7 +14,7 @@ * integration, Inspector support, and remote and inline image * handling. */ -public abstract class ClientWebView : WebKit.WebView, Geary.BaseInterface { +public abstract class Components.WebView : WebKit.WebView, Geary.BaseInterface { /** URI Scheme and delimiter for internal resource loads. */ @@ -65,7 +65,6 @@ public abstract class ClientWebView : WebKit.WebView, Geary.BaseInterface { private static WebKit.UserStyleSheet? user_stylesheet = null; private static WebKit.UserScript? script = null; - private static WebKit.UserScript? allow_remote_images = null; /** @@ -76,23 +75,18 @@ public abstract class ClientWebView : WebKit.WebView, Geary.BaseInterface { File cache_dir) { WebsiteDataManager data_manager = new WebsiteDataManager(cache_dir.get_path()); WebKit.WebContext context = new WebKit.WebContext.with_website_data_manager(data_manager); -#if HAS_WEBKIT_SHARED_PROC - // Use a shared process so we don't spawn N WebProcess instances - // when showing N messages in a conversation. - context.set_process_model(WebKit.ProcessModel.SHARED_SECONDARY_PROCESS); -#endif // Use the doc viewer model since each web view instance only // ever shows a single HTML document. context.set_cache_model(WebKit.CacheModel.DOCUMENT_VIEWER); context.register_uri_scheme("cid", (req) => { - ClientWebView? view = req.get_web_view() as ClientWebView; + WebView? view = req.get_web_view() as WebView; if (view != null) { view.handle_cid_request(req); } }); context.register_uri_scheme("geary", (req) => { - ClientWebView? view = req.get_web_view() as ClientWebView; + WebView? view = req.get_web_view() as WebView; if (view != null) { view.handle_internal_request(req); } @@ -113,25 +107,22 @@ public abstract class ClientWebView : WebKit.WebView, Geary.BaseInterface { update_spellcheck(context, config); }); - ClientWebView.default_context = context; + WebView.default_context = context; } /** - * Loads static resources used by ClientWebView. + * Loads static resources used by WebView. */ public static void load_resources(GLib.File user_dir) throws GLib.Error { - ClientWebView.script = load_app_script( - "client-web-view.js" - ); - ClientWebView.allow_remote_images = load_app_script( - "client-web-view-allow-remote-images.js" + WebView.script = load_app_script( + "components-web-view.js" ); foreach (string name in new string[] { USER_CSS, USER_CSS_LEGACY }) { GLib.File stylesheet = user_dir.get_child(name); try { - ClientWebView.user_stylesheet = load_user_stylesheet(stylesheet); + WebView.user_stylesheet = load_user_stylesheet(stylesheet); break; } catch (GLib.IOError.NOT_FOUND err) { // All good, try the next one or just exit @@ -299,8 +290,9 @@ public abstract class ClientWebView : WebKit.WebView, Geary.BaseInterface { public signal void remote_image_load_blocked(); - protected ClientWebView(Application.Configuration config, - WebKit.UserContentManager? custom_manager = null) { + protected WebView(Application.Configuration config, + WebKit.UserContentManager? custom_manager = null, + WebView? related = null) { WebKit.Settings setts = new WebKit.Settings(); setts.allow_modal_dialogs = false; setts.default_charset = "UTF-8"; @@ -321,62 +313,40 @@ public abstract class ClientWebView : WebKit.WebView, Geary.BaseInterface { WebKit.UserContentManager content_manager = custom_manager ?? new WebKit.UserContentManager(); - content_manager.add_script(ClientWebView.script); - if (ClientWebView.user_stylesheet != null) { - content_manager.add_style_sheet(ClientWebView.user_stylesheet); + content_manager.add_script(WebView.script); + if (WebView.user_stylesheet != null) { + content_manager.add_style_sheet(WebView.user_stylesheet); } Object( - web_context: ClientWebView.default_context, + settings: setts, user_content_manager: content_manager, - settings: setts + web_context: WebView.default_context ); base_ref(); - - // XXX get the allow prefix from the extension somehow - - this.decide_policy.connect(on_decide_policy); - this.web_process_terminated.connect((reason) => { - warning("Web process crashed: %s", reason.to_string()); - }); - - register_message_handler( - COMMAND_STACK_CHANGED, on_command_stack_changed - ); - register_message_handler( - CONTENT_LOADED, on_content_loaded - ); - register_message_handler( - DOCUMENT_MODIFIED, on_document_modified - ); - register_message_handler( - PREFERRED_HEIGHT_CHANGED, on_preferred_height_changed - ); - register_message_handler( - REMOTE_IMAGE_LOAD_BLOCKED, on_remote_image_load_blocked - ); - register_message_handler( - SELECTION_CHANGED, on_selection_changed - ); - - // Manage zoom level, ensure it's sane - config.bind(Application.Configuration.CONVERSATION_VIEWER_ZOOM_KEY, this, "zoom_level"); - if (this.zoom_level < ZOOM_MIN) { - this.zoom_level = ZOOM_MIN; - } else if (this.zoom_level > ZOOM_MAX) { - this.zoom_level = ZOOM_MAX; - } - this.scroll_event.connect(on_scroll_event); - - // Watch desktop font settings - Settings system_settings = config.gnome_interface; - system_settings.bind("document-font-name", this, - "document-font", SettingsBindFlags.DEFAULT); - system_settings.bind("monospace-font-name", this, - "monospace-font", SettingsBindFlags.DEFAULT); + init(config); } - ~ClientWebView() { + /** + * Constructs a new web view with a new shared WebProcess. + * + * The new view will use the same WebProcess, settings and content + * manager as the given related view's. + * + * @see WebKit.WebView.with_related_view + */ + protected WebView.with_related_view(Application.Configuration config, + WebView related) { + Object( + related_view: related, + settings: related.get_settings(), + user_content_manager: related.user_content_manager + ); + base_ref(); + init(config); + } + + ~WebView() { base_unref(); } @@ -433,13 +403,7 @@ public abstract class ClientWebView : WebKit.WebView, Geary.BaseInterface { * effect. */ public void allow_remote_image_loading() { - // Use a separate script here since we need to update the - // value of window.geary.allow_remote_image_loading after it - // was first created by client-web-view.js (which is loaded at - // the start of page load), but before the page load is - // started (so that any remote images present are actually - // loaded). - this.user_content_manager.add_script(ClientWebView.allow_remote_images); + this.run_javascript.begin("_gearyAllowRemoteResourceLoads = true", null); } /** @@ -515,7 +479,7 @@ public abstract class ClientWebView : WebKit.WebView, Geary.BaseInterface { JavaScriptMessageHandler handler) { // XXX can't use the delegate directly, see b.g.o Bug // 604781. However the workaround below creates a circular - // reference, causing ClientWebView instances to leak. So to + // reference, causing WebView instances to leak. So to // work around that we need to record handler ids and // disconnect them when being destroyed. ulong id = this.user_content_manager.script_message_received[name].connect( @@ -527,6 +491,50 @@ public abstract class ClientWebView : WebKit.WebView, Geary.BaseInterface { } } + private void init(Application.Configuration config) { + // XXX get the allow prefix from the extension somehow + + this.decide_policy.connect(on_decide_policy); + this.web_process_terminated.connect((reason) => { + warning("Web process crashed: %s", reason.to_string()); + }); + + register_message_handler( + COMMAND_STACK_CHANGED, on_command_stack_changed + ); + register_message_handler( + CONTENT_LOADED, on_content_loaded + ); + register_message_handler( + DOCUMENT_MODIFIED, on_document_modified + ); + register_message_handler( + PREFERRED_HEIGHT_CHANGED, on_preferred_height_changed + ); + register_message_handler( + REMOTE_IMAGE_LOAD_BLOCKED, on_remote_image_load_blocked + ); + register_message_handler( + SELECTION_CHANGED, on_selection_changed + ); + + // Manage zoom level, ensure it's sane + config.bind(Application.Configuration.CONVERSATION_VIEWER_ZOOM_KEY, this, "zoom_level"); + if (this.zoom_level < ZOOM_MIN) { + this.zoom_level = ZOOM_MIN; + } else if (this.zoom_level > ZOOM_MAX) { + this.zoom_level = ZOOM_MAX; + } + this.scroll_event.connect(on_scroll_event); + + // Watch desktop font settings + Settings system_settings = config.gnome_interface; + system_settings.bind("document-font-name", this, + "document-font", SettingsBindFlags.DEFAULT); + system_settings.bind("monospace-font-name", this, + "monospace-font", SettingsBindFlags.DEFAULT); + } + private void handle_cid_request(WebKit.URISchemeRequest request) { if (!handle_internal_response(request)) { request.finish_error(new FileError.NOENT("Unknown CID")); diff --git a/src/client/composer/composer-web-view.vala b/src/client/composer/composer-web-view.vala index aea46978..f8ecccf6 100644 --- a/src/client/composer/composer-web-view.vala +++ b/src/client/composer/composer-web-view.vala @@ -9,7 +9,7 @@ /** * A WebView for editing messages in the composer. */ -public class Composer.WebView : ClientWebView { +public class Composer.WebView : Components.WebView { /** HTML id used for the main text section of the message body. */ public const string BODY_HTML_ID = "geary-body"; @@ -112,10 +112,10 @@ public class Composer.WebView : ClientWebView { public static new void load_resources() throws Error { - WebView.app_style = ClientWebView.load_app_stylesheet( + WebView.app_style = Components.WebView.load_app_stylesheet( "composer-web-view.css" ); - WebView.app_script = ClientWebView.load_app_script( + WebView.app_script = Components.WebView.load_app_script( "composer-web-view.js" ); } diff --git a/src/client/composer/composer-widget.vala b/src/client/composer/composer-widget.vala index 17430021..4c4d0caf 100644 --- a/src/client/composer/composer-widget.vala +++ b/src/client/composer/composer-widget.vala @@ -1252,7 +1252,7 @@ public class Composer.Widget : Gtk.EventBox, Geary.BaseInterface { email.inline_files.set_all(this.inline_files); email.cid_files.set_all(this.cid_files); - email.img_src_prefix = ClientWebView.INTERNAL_URL_PREFIX; + email.img_src_prefix = Components.WebView.INTERNAL_URL_PREFIX; try { email.body_text = yield this.editor.body.get_text(); @@ -1924,7 +1924,7 @@ public class Composer.Widget : Gtk.EventBox, Geary.BaseInterface { string unique_filename; add_inline_part(byte_buffer, filename, out unique_filename); this.editor.body.insert_image( - ClientWebView.INTERNAL_URL_PREFIX + unique_filename + Components.WebView.INTERNAL_URL_PREFIX + unique_filename ); } catch (Error error) { this.application.report_problem( @@ -1964,7 +1964,7 @@ public class Composer.Widget : Gtk.EventBox, Geary.BaseInterface { string unique_filename; add_inline_part(file_buffer, path, out unique_filename); this.editor.body.insert_image( - ClientWebView.INTERNAL_URL_PREFIX + unique_filename + Components.WebView.INTERNAL_URL_PREFIX + unique_filename ); } catch (Error err) { attachment_failed(err.message); @@ -2459,7 +2459,7 @@ public class Composer.Widget : Gtk.EventBox, Geary.BaseInterface { } this.editor.body.insert_image( - ClientWebView.INTERNAL_URL_PREFIX + unique_filename + Components.WebView.INTERNAL_URL_PREFIX + unique_filename ); } diff --git a/src/client/conversation-viewer/conversation-email.vala b/src/client/conversation-viewer/conversation-email.vala index e9d09286..868f33a3 100644 --- a/src/client/conversation-viewer/conversation-email.vala +++ b/src/client/conversation-viewer/conversation-email.vala @@ -462,7 +462,7 @@ public class ConversationEmail : Gtk.Box, Geary.BaseInterface { if (this.body_selection_message != null) { try { selection = - yield this.body_selection_message.web_view.get_selection_for_quoting(); + yield this.body_selection_message.get_selection_for_quoting(); } catch (Error err) { debug("Failed to get selection for quoting: %s", err.message); } @@ -478,7 +478,7 @@ public class ConversationEmail : Gtk.Box, Geary.BaseInterface { if (this.body_selection_message != null) { try { selection = - yield this.body_selection_message.web_view.get_selection_for_find(); + yield this.body_selection_message.get_selection_for_find(); } catch (Error err) { debug("Failed to get selection for find: %s", err.message); } @@ -571,12 +571,10 @@ public class ConversationEmail : Gtk.Box, Geary.BaseInterface { Json.Generator generator = new Json.Generator(); generator.set_root(builder.get_root()); string js = "geary.addPrintHeaders(" + generator.to_data(null) + ");"; - yield this.primary_message.web_view.run_javascript(js, null); + yield this.primary_message.run_javascript(js, null); Gtk.Window? window = get_toplevel() as Gtk.Window; - WebKit.PrintOperation op = new WebKit.PrintOperation( - this.primary_message.web_view - ); + WebKit.PrintOperation op = this.primary_message.new_print_operation(); Gtk.PrintSettings settings = new Gtk.PrintSettings(); if (this.email.subject != null) { @@ -603,14 +601,14 @@ public class ConversationEmail : Gtk.Box, Geary.BaseInterface { } private void connect_message_view_signals(ConversationMessage view) { + view.content_loaded.connect(on_content_loaded); view.flag_remote_images.connect(on_flag_remote_images); view.internal_link_activated.connect((y) => { internal_link_activated(y); }); + view.internal_resource_loaded.connect(on_resource_loaded); view.save_image.connect(on_save_image); - view.web_view.internal_resource_loaded.connect(on_resource_loaded); - view.web_view.content_loaded.connect(on_content_loaded); - view.web_view.selection_changed.connect((has_selection) => { + view.selection_changed.connect((has_selection) => { this.body_selection_message = has_selection ? view : null; body_selection_changed(has_selection); }); @@ -686,7 +684,7 @@ public class ConversationEmail : Gtk.Box, Geary.BaseInterface { // Load all messages - this.primary_message.web_view.add_internal_resources(cid_resources); + this.primary_message.add_internal_resources(cid_resources); yield this.primary_message.load_message_body( message, this.load_cancellable ); @@ -704,7 +702,7 @@ public class ConversationEmail : Gtk.Box, Geary.BaseInterface { this.config ); connect_message_view_signals(attached_message); - attached_message.web_view.add_internal_resources(cid_resources); + attached_message.add_internal_resources(cid_resources); this.sub_messages.add(attached_message); this._attached_messages.add(attached_message); attached_message.load_contacts.begin(this.load_cancellable); @@ -886,8 +884,8 @@ public class ConversationEmail : Gtk.Box, Geary.BaseInterface { Geary.Memory.Buffer? content) { var main = get_toplevel() as Application.MainWindow; if (main != null) { - if (uri.has_prefix(ClientWebView.CID_URL_PREFIX)) { - string cid = uri.substring(ClientWebView.CID_URL_PREFIX.length); + if (uri.has_prefix(Components.WebView.CID_URL_PREFIX)) { + string cid = uri.substring(Components.WebView.CID_URL_PREFIX.length); try { Geary.Attachment attachment = this.email.get_attachment_by_content_id( cid @@ -934,7 +932,7 @@ public class ConversationEmail : Gtk.Box, Geary.BaseInterface { private void on_content_loaded() { bool all_loaded = true; foreach (ConversationMessage message in this) { - if (!message.web_view.is_content_loaded) { + if (!message.is_content_loaded) { all_loaded = false; break; } diff --git a/src/client/conversation-viewer/conversation-list-box.vala b/src/client/conversation-viewer/conversation-list-box.vala index 005b8f10..3eb8240b 100644 --- a/src/client/conversation-viewer/conversation-list-box.vala +++ b/src/client/conversation-viewer/conversation-list-box.vala @@ -957,7 +957,7 @@ public class ConversationListBox : Gtk.ListBox, Geary.BaseInterface { */ public void zoom_in() { message_view_iterator().foreach((msg_view) => { - msg_view.web_view.zoom_in(); + msg_view.zoom_in(); return true; }); } @@ -967,7 +967,7 @@ public class ConversationListBox : Gtk.ListBox, Geary.BaseInterface { */ public void zoom_out() { message_view_iterator().foreach((msg_view) => { - msg_view.web_view.zoom_out(); + msg_view.zoom_out(); return true; }); } @@ -977,7 +977,7 @@ public class ConversationListBox : Gtk.ListBox, Geary.BaseInterface { */ public void zoom_reset() { message_view_iterator().foreach((msg_view) => { - msg_view.web_view.zoom_reset(); + msg_view.zoom_reset(); return true; }); } @@ -1182,8 +1182,7 @@ public class ConversationListBox : Gtk.ListBox, Geary.BaseInterface { row.get_allocation(out alloc); int x = 0, y = 0; - ConversationWebView web_view = row.view.primary_message.web_view; - web_view.translate_coordinates(row, x, anchor_y, out x, out y); + row.view.primary_message.web_view_translate_coordinates(row, x, anchor_y, out x, out y); Gtk.Adjustment adj = get_adjustment(); y = alloc.y + y; @@ -1216,14 +1215,13 @@ public class ConversationListBox : Gtk.ListBox, Geary.BaseInterface { ConversationMessage conversation_message = view.primary_message; int body_top = 0; int body_left = 0; - ConversationWebView web_view = conversation_message.web_view; - web_view.translate_coordinates( + conversation_message.web_view_translate_coordinates( this, 0, 0, out body_left, out body_top ); - int body_height = web_view.get_allocated_height(); + int body_height = conversation_message.web_view_get_allocated_height(); int body_bottom = body_top + body_height; // Only mark the email as read if it's actually visible diff --git a/src/client/conversation-viewer/conversation-message.vala b/src/client/conversation-viewer/conversation-message.vala index 50ac031e..a11ece01 100644 --- a/src/client/conversation-viewer/conversation-message.vala +++ b/src/client/conversation-viewer/conversation-message.vala @@ -310,8 +310,19 @@ public class ConversationMessage : Gtk.Grid, Geary.BaseInterface { [GtkChild] internal Components.InfoBarStack info_bars; + /** + * Emitted when web_view's content has finished loaded. + * + * See {@link Components.WebView.is_content_loaded} for details. + */ + internal bool is_content_loaded { + get { + return this.web_view != null && this.web_view.is_content_loaded; + } + } + /** HTML view that displays the message body. */ - internal ConversationWebView web_view { get; private set; } + private ConversationWebView? web_view { get; private set; } // The message headers represented by this view private Geary.EmailHeaderSet headers; @@ -426,6 +437,19 @@ public class ConversationMessage : Gtk.Grid, Geary.BaseInterface { string uri, string? alt_text, Geary.Memory.Buffer? buffer ); + /** Emitted when web_view has loaded a resource added to it. */ + public signal void internal_resource_loaded(string name); + + /** Emitted when web_view's selection has changed. */ + public signal void selection_changed(bool has_selection); + + /** + * Emitted when web_view's content has finished loaded. + * + * See {@link Components.WebView.is_content_loaded} for details. + */ + public signal void content_loaded(); + /** * Constructs a new view from an email's headers and body. @@ -467,6 +491,18 @@ public class ConversationMessage : Gtk.Grid, Geary.BaseInterface { ); } + private void trigger_internal_resource_loaded(string name) { + internal_resource_loaded(name); + } + + private void trigger_content_loaded() { + content_loaded(); + } + + private void trigger_selection_changed(bool has_selection) { + selection_changed(has_selection); + } + private ConversationMessage(Geary.EmailHeaderSet headers, string? preview, bool load_remote_resources, @@ -487,19 +523,10 @@ public class ConversationMessage : Gtk.Grid, Geary.BaseInterface { .activate.connect(on_copy_email_address); add_action(ACTION_COPY_LINK, true, VariantType.STRING) .activate.connect(on_copy_link); - add_action(ACTION_COPY_SELECTION, false).activate.connect(() => { - web_view.copy_clipboard(); - }); - add_action(ACTION_OPEN_INSPECTOR, config.enable_inspector).activate.connect(() => { - this.web_view.get_inspector().show(); - }); add_action(ACTION_OPEN_LINK, true, VariantType.STRING) .activate.connect(on_link_activated); add_action(ACTION_SAVE_IMAGE, true, new VariantType("(sms)")) .activate.connect(on_save_image); - add_action(ACTION_SELECT_ALL, true).activate.connect(() => { - web_view.select_all(); - }); insert_action_group("msg", message_actions); // Context menu @@ -552,25 +579,7 @@ public class ConversationMessage : Gtk.Grid, Geary.BaseInterface { this.subject_searchable = headers.subject.value.casefold(); } - // Web view - - this.web_view = new ConversationWebView(config); - this.web_view.context_menu.connect(on_context_menu); - this.web_view.deceptive_link_clicked.connect(on_deceptive_link_clicked); - this.web_view.link_activated.connect((link) => { - on_link_activated(new GLib.Variant("s", link)); - }); - this.web_view.mouse_target_changed.connect(on_mouse_target_changed); - this.web_view.notify["is-loading"].connect(on_is_loading_notify); - this.web_view.resource_load_started.connect(on_resource_load_started); - this.web_view.remote_image_load_blocked.connect(on_remote_images_blocked); - this.web_view.selection_changed.connect(on_selection_changed); - this.web_view.set_hexpand(true); - this.web_view.set_vexpand(true); - this.web_view.show(); - this.body_container.set_has_tooltip(true); // Used to show link URLs - this.body_container.add(this.web_view); this.show_progress_timeout = new Geary.TimeoutManager.milliseconds( Util.Gtk.SHOW_PROGRESS_TIMEOUT_MSEC, this.on_show_progress_timeout ); @@ -584,6 +593,51 @@ public class ConversationMessage : Gtk.Grid, Geary.BaseInterface { this.progress_pulse.repetition = FOREVER; } + private void initialize_web_view() { + var viewer = get_ancestor(typeof(ConversationViewer)) as ConversationViewer; + + // Ensure we share the same WebProcess with the last one + // constructed if possible. + if (viewer != null && viewer.previous_web_view != null) { + this.web_view = new ConversationWebView.with_related_view( + this.config, + viewer.previous_web_view + ); + } else { + this.web_view = new ConversationWebView(this.config); + } + if (viewer != null) { + viewer.previous_web_view = this.web_view; + } + + this.web_view.context_menu.connect(on_context_menu); + this.web_view.deceptive_link_clicked.connect(on_deceptive_link_clicked); + this.web_view.link_activated.connect((link) => { + on_link_activated(new GLib.Variant("s", link)); + }); + this.web_view.mouse_target_changed.connect(on_mouse_target_changed); + this.web_view.notify["is-loading"].connect(on_is_loading_notify); + this.web_view.resource_load_started.connect(on_resource_load_started); + this.web_view.remote_image_load_blocked.connect(on_remote_images_blocked); + this.web_view.selection_changed.connect(on_selection_changed); + this.web_view.internal_resource_loaded.connect(trigger_internal_resource_loaded); + this.web_view.content_loaded.connect(trigger_content_loaded); + this.web_view.selection_changed.connect(trigger_selection_changed); + this.web_view.set_hexpand(true); + this.web_view.set_vexpand(true); + this.web_view.show(); + this.body_container.add(this.web_view); + add_action(ACTION_COPY_SELECTION, false).activate.connect(() => { + web_view.copy_clipboard(); + }); + add_action(ACTION_OPEN_INSPECTOR, config.enable_inspector).activate.connect(() => { + this.web_view.get_inspector().show(); + }); + add_action(ACTION_SELECT_ALL, true).activate.connect(() => { + web_view.select_all(); + }); + } + ~ConversationMessage() { base_unref(); } @@ -597,10 +651,77 @@ public class ConversationMessage : Gtk.Grid, Geary.BaseInterface { base.destroy(); } + public async string? get_selection_for_quoting() throws Error { + if (this.web_view == null) + initialize_web_view(); + return yield web_view.get_selection_for_quoting(); + } + + public async string? get_selection_for_find() throws Error { + if (this.web_view == null) + initialize_web_view(); + return yield web_view.get_selection_for_find(); + } + + /** + * Adds a set of internal resources to web_view. + * + * @see add_internal_resource + */ + public void add_internal_resources(Gee.Map res) { + if (this.web_view == null) + initialize_web_view(); + web_view.add_internal_resources(res); + } + + public WebKit.PrintOperation new_print_operation() { + if (this.web_view == null) + initialize_web_view(); + return new WebKit.PrintOperation(web_view); + } + + public async void run_javascript (string script, Cancellable? cancellable) throws Error { + if (this.web_view == null) + initialize_web_view(); + yield web_view.run_javascript(script, cancellable); + } + + public void zoom_in() { + if (this.web_view == null) + initialize_web_view(); + web_view.zoom_in(); + } + + public void zoom_out() { + if (this.web_view == null) + initialize_web_view(); + web_view.zoom_out(); + } + + public void zoom_reset() { + if (this.web_view == null) + initialize_web_view(); + web_view.zoom_reset(); + } + + public void web_view_translate_coordinates(Gtk.Widget widget, int x, int anchor_y, out int x1, out int y1) { + if (this.web_view == null) + initialize_web_view(); + web_view.translate_coordinates(widget, x, anchor_y, out x1, out y1); + } + + public int web_view_get_allocated_height() { + if (this.web_view == null) + initialize_web_view(); + return web_view.get_allocated_height(); + } + /** * Shows the complete message and hides the compact headers. */ public void show_message_body(bool include_transitions=true) { + if (this.web_view == null) + initialize_web_view(); set_revealer(this.compact_revealer, false, include_transitions); set_revealer(this.header_revealer, true, include_transitions); set_revealer(this.body_revealer, true, include_transitions); @@ -785,6 +906,10 @@ public class ConversationMessage : Gtk.Grid, Geary.BaseInterface { throw new GLib.IOError.CANCELLED("Conversation load cancelled"); } + if (this.web_view == null) { + initialize_web_view(); + } + bool contact_load_images = ( this.primary_contact != null && this.primary_contact.load_remote_resources @@ -835,6 +960,8 @@ public class ConversationMessage : Gtk.Grid, Geary.BaseInterface { } } + if (this.web_view == null) + initialize_web_view(); uint webkit_found = yield this.web_view.highlight_search_terms( search_matches, cancellable ); @@ -848,7 +975,9 @@ public class ConversationMessage : Gtk.Grid, Geary.BaseInterface { foreach (ContactFlowBoxChild address in this.searchable_addresses) { address.unmark_search_terms(); } - this.web_view.unmark_search_terms(); + + if (this.web_view != null) + this.web_view.unmark_search_terms(); } /** @@ -1011,6 +1140,8 @@ public class ConversationMessage : Gtk.Grid, Geary.BaseInterface { // returns HTML that is placed into the document in the position // where the MIME part was found private string? inline_image_replacer(Geary.RFC822.Part part) { + if (this.web_view == null) + initialize_web_view(); Geary.Mime.ContentType content_type = part.content_type; if (content_type.media_type != "image" || !this.web_view.can_show_mime_type(content_type.to_string())) { @@ -1045,7 +1176,7 @@ public class ConversationMessage : Gtk.Grid, Geary.BaseInterface { return "\"%s\"".printf( clean_filename, REPLACED_IMAGE_CLASS, - ClientWebView.CID_URL_PREFIX, + Components.WebView.CID_URL_PREFIX, Geary.HTML.escape_markup(id) ); } @@ -1059,7 +1190,9 @@ public class ConversationMessage : Gtk.Grid, Geary.BaseInterface { this.load_remote_resources = true; this.remote_resources_requested = 0; this.remote_resources_loaded = 0; - this.web_view.load_remote_images(); + if (this.web_view != null) { + this.web_view.load_remote_images(); + } if (update_email_flag) { flag_remote_images(); } @@ -1074,11 +1207,13 @@ public class ConversationMessage : Gtk.Grid, Geary.BaseInterface { if (placeholder != null) { this.body_placeholder = placeholder; - this.web_view.hide(); + if (this.web_view != null) + this.web_view.hide(); this.body_container.add(placeholder); show_message_body(true); } else { - this.web_view.show(); + if (this.web_view != null) + this.web_view.show(); } } @@ -1106,10 +1241,12 @@ public class ConversationMessage : Gtk.Grid, Geary.BaseInterface { } private void on_is_loading_notify() { - if (this.web_view.is_loading) { - start_progress_loading(); - } else { - stop_progress_loading(); + if (this.web_view != null) { + if (this.web_view.is_loading) { + start_progress_loading(); + } else { + stop_progress_loading(); + } } } @@ -1369,7 +1506,7 @@ public class ConversationMessage : Gtk.Grid, Geary.BaseInterface { alt_text = (string) alt_maybe; } - if (uri.has_prefix(ClientWebView.CID_URL_PREFIX)) { + if (uri.has_prefix(Components.WebView.CID_URL_PREFIX)) { // We can get the data directly from the attachment, so // don't bother getting it from the web view save_image(uri, alt_text, null); diff --git a/src/client/conversation-viewer/conversation-viewer.vala b/src/client/conversation-viewer/conversation-viewer.vala index eec5f6a4..a5098764 100644 --- a/src/client/conversation-viewer/conversation-viewer.vala +++ b/src/client/conversation-viewer/conversation-viewer.vala @@ -24,6 +24,14 @@ public class ConversationViewer : Gtk.Stack, Geary.BaseInterface { get; private set; default = null; } + /** + * The most recent web view created in this viewer. + * + * Keep the last created web view around so others can share the + * same WebKitGTK WebProcess. + */ + internal ConversationWebView? previous_web_view { get; set; default = null; } + private Application.Configuration config; private Gee.Set? selection_while_composing = null; @@ -251,7 +259,10 @@ public class ConversationViewer : Gtk.Stack, Geary.BaseInterface { Application.ContactStore contacts, bool start_mark_timer) throws GLib.Error { - remove_current_list(); + // Keep the old ScrolledWindow around long enough for its + // descendant web views to be kept so their WebProcess can be + // re-used. + var old_scroller = remove_current_list(); ConversationListBox new_list = new ConversationListBox( conversation, @@ -297,6 +308,9 @@ public class ConversationViewer : Gtk.Stack, Geary.BaseInterface { } yield new_list.load_conversation(scroll_to, query); + + // Not strictly necessary, but keeps the compiler happy + old_scroller.destroy(); } // Add a new conversation list to the UI @@ -316,7 +330,7 @@ public class ConversationViewer : Gtk.Stack, Geary.BaseInterface { } // Remove any existing conversation list, cancelling its loading - private void remove_current_list() { + private Gtk.ScrolledWindow remove_current_list() { if (this.find_cancellable != null) { this.find_cancellable.cancel(); this.find_cancellable = null; @@ -328,15 +342,17 @@ public class ConversationViewer : Gtk.Stack, Geary.BaseInterface { this.current_list = null; } + var old_scroller = this.conversation_scroller; // XXX GTK+ Bug 778190 workaround - this.conversation_scroller.destroy(); // removes the list + this.conversation_page.remove(old_scroller); new_conversation_scroller(); + return old_scroller; } private void new_conversation_scroller() { // XXX Work around for GTK+ Bug 778190: Instead of replacing // the Viewport that contains the current list, replace the - // complete ScrolledWindow. Need to put remove this method and + // complete ScrolledWindow. Need to remove this method and // put the settings back into conversation-viewer.ui when we // can rely on it being fixed again. Gtk.ScrolledWindow scroller = new Gtk.ScrolledWindow(null, null); diff --git a/src/client/conversation-viewer/conversation-web-view.vala b/src/client/conversation-viewer/conversation-web-view.vala index 70e5c633..ffa36394 100644 --- a/src/client/conversation-viewer/conversation-web-view.vala +++ b/src/client/conversation-viewer/conversation-web-view.vala @@ -6,7 +6,7 @@ * (version 2.1 or later). See the COPYING file in this distribution. */ -public class ConversationWebView : ClientWebView { +public class ConversationWebView : Components.WebView { private const string DECEPTIVE_LINK_CLICKED = "deceptiveLinkClicked"; @@ -41,10 +41,10 @@ public class ConversationWebView : ClientWebView { public static new void load_resources() throws Error { - ConversationWebView.app_script = ClientWebView.load_app_script( + ConversationWebView.app_script = Components.WebView.load_app_script( "conversation-web-view.js" ); - ConversationWebView.app_stylesheet = ClientWebView.load_app_stylesheet( + ConversationWebView.app_stylesheet = Components.WebView.load_app_stylesheet( "conversation-web-view.css" ); } @@ -56,16 +56,33 @@ public class ConversationWebView : ClientWebView { ); + /** + * Constructs a new web view for displaying an email message body. + * + * A new WebKitGTK WebProcess will be constructed for this view. + */ public ConversationWebView(Application.Configuration config) { base(config); + init(); + + // These only need to be added when creating a new WebProcess, + // not when sharing one this.user_content_manager.add_script(ConversationWebView.app_script); this.user_content_manager.add_style_sheet(ConversationWebView.app_stylesheet); + } - register_message_handler( - DECEPTIVE_LINK_CLICKED, on_deceptive_link_clicked - ); - - this.notify["preferred-height"].connect(() => queue_resize()); + /** + * Constructs a new web view for displaying an email message body. + * + * The WebKitGTK WebProcess will be shared with the related view's + * process. + */ + internal ConversationWebView.with_related_view( + Application.Configuration config, + ConversationWebView related + ) { + base.with_related_view(config, related); + init(); } /** @@ -206,6 +223,14 @@ public class ConversationWebView : ClientWebView { minimum_height = natural_height = 0; } + private void init() { + register_message_handler( + DECEPTIVE_LINK_CLICKED, on_deceptive_link_clicked + ); + + this.notify["preferred-height"].connect(() => queue_resize()); + } + private void on_deceptive_link_clicked(WebKit.JavascriptResult result) { try { JSC.Value object = result.get_js_value(); diff --git a/src/client/meson.build b/src/client/meson.build index c0eb0c16..335964f1 100644 --- a/src/client/meson.build +++ b/src/client/meson.build @@ -46,7 +46,6 @@ client_vala_sources = files( 'client-action.vala', - 'components/client-web-view.vala', 'components/components-attachment-pane.vala', 'components/components-conversation-actions.vala', 'components/components-conversation-action-bar.vala', @@ -64,6 +63,7 @@ client_vala_sources = files( 'components/components-reflow-box.c', 'components/components-search-bar.vala', 'components/components-validator.vala', + 'components/components-web-view.vala', 'components/count-badge.vala', 'components/folder-popover.vala', 'components/icon-factory.vala', @@ -191,16 +191,6 @@ client_vala_args += [ ) ] -# Enable shared shecondary process if available. -# See issues #558 and #559 -webkit_version = webkit2gtk.version().split('.') -if webkit_version[0].to_int() <= 2 and webkit_version[1].to_int() <= 24 - message('Enabling WebKitGTK shared process model') - client_vala_args += [ - '-D', 'HAS_WEBKIT_SHARED_PROC' - ] -endif - # Main client application library client_lib = shared_library( client_package, diff --git a/src/client/web-process/web-process-extension.vala b/src/client/web-process/web-process-extension.vala index b2b29bf9..4bba5154 100644 --- a/src/client/web-process/web-process-extension.vala +++ b/src/client/web-process/web-process-extension.vala @@ -32,22 +32,14 @@ public class GearyWebExtension : Object { private const string[] ALLOWED_SCHEMES = { "cid", "geary", "data", "blob" }; + private const string REMOTE_LOAD_VAR = "_gearyAllowRemoteResourceLoads"; + private WebKit.WebExtension extension; public GearyWebExtension(WebKit.WebExtension extension) { this.extension = extension; - extension.page_created.connect((extension, web_page) => { - web_page.console_message_sent.connect(on_console_message); - web_page.send_request.connect(on_send_request); - // XXX investigate whether the earliest supported - // version of WK supports the DOM "selectionchanged" - // event, and if so use that rather that doing it in - // here in the extension - web_page.get_editor().selection_changed.connect(() => { - selection_changed(web_page); - }); - }); + extension.page_created.connect(on_page_created); } // XXX Conditionally enable while we still depend on WK2 <2.12 @@ -89,14 +81,7 @@ public class GearyWebExtension : Object { WebKit.Frame frame = page.get_main_frame(); JSC.Context context = frame.get_js_context(); try { - JSC.Value ret = execute_script( - context, - "geary.allowRemoteImages", - GLib.Log.FILE, - GLib.Log.METHOD, - GLib.Log.LINE - ); - should_load = Util.JS.to_bool(ret); + should_load = Util.JS.to_bool(context.get_value(REMOTE_LOAD_VAR)); } catch (GLib.Error err) { debug( "Error checking PageState::allowRemoteImages: %s", @@ -154,4 +139,24 @@ public class GearyWebExtension : Object { return ret; } + private void on_page_created(WebKit.WebExtension extension, + WebKit.WebPage page) { + WebKit.Frame frame = page.get_main_frame(); + JSC.Context context = frame.get_js_context(); + context.set_value( + REMOTE_LOAD_VAR, + new JSC.Value.boolean(context, false) + ); + + page.console_message_sent.connect(on_console_message); + page.send_request.connect(on_send_request); + // XXX investigate whether the earliest supported + // version of WK supports the DOM "selectionchanged" + // event, and if so use that rather that doing it in + // here in the extension + page.get_editor().selection_changed.connect(() => { + selection_changed(page); + }); + } + } diff --git a/test/client/components/client-web-view-test-case.vala b/test/client/components/components-web-view-test-case.vala similarity index 74% rename from test/client/components/client-web-view-test-case.vala rename to test/client/components/components-web-view-test-case.vala index 10d21b1f..04f1c79f 100644 --- a/test/client/components/client-web-view-test-case.vala +++ b/test/client/components/components-web-view-test-case.vala @@ -6,35 +6,42 @@ */ -public abstract class ClientWebViewTestCase : TestCase { +public abstract class Components.WebViewTestCase : TestCase { protected V? test_view = null; protected Application.Configuration? config = null; - protected ClientWebViewTestCase(string name) { + protected WebViewTestCase(string name) { base(name); + } + + public override void set_up() { this.config = new Application.Configuration(Application.Client.SCHEMA_ID); this.config.enable_debug = true; - ClientWebView.init_web_context( + + WebView.init_web_context( this.config, File.new_for_path(_BUILD_ROOT_DIR).get_child("src"), File.new_for_path("/tmp") // XXX use something better here ); try { - ClientWebView.load_resources(GLib.File.new_for_path("/tmp")); + WebView.load_resources(GLib.File.new_for_path("/tmp")); } catch (GLib.Error err) { GLib.assert_not_reached(); } + + this.test_view = set_up_test_view(); } - public override void set_up() { - this.test_view = set_up_test_view(); + protected override void tear_down() { + this.config = null; + this.test_view = null; } protected abstract V set_up_test_view(); protected virtual void load_body_fixture(string html = "") { - ClientWebView client_view = (ClientWebView) this.test_view; + WebView client_view = (WebView) this.test_view; client_view.load_html(html); while (!client_view.is_content_loaded) { Gtk.main_iteration(); @@ -42,7 +49,7 @@ public abstract class ClientWebViewTestCase : TestCase { } protected WebKit.JavascriptResult run_javascript(string command) throws Error { - ClientWebView view = (ClientWebView) this.test_view; + WebView view = (WebView) this.test_view; view.run_javascript.begin(command, null, this.async_completion); return view.run_javascript.end(async_result()); } diff --git a/test/client/components/client-web-view-test.vala b/test/client/components/components-web-view-test.vala similarity index 79% rename from test/client/components/client-web-view-test.vala rename to test/client/components/components-web-view-test.vala index d08e5192..d3e1037c 100644 --- a/test/client/components/client-web-view-test.vala +++ b/test/client/components/components-web-view-test.vala @@ -5,10 +5,10 @@ * (version 2.1 or later). See the COPYING file in this distribution. */ -public class ClientWebViewTest : TestCase { +public class Components.WebViewTest : TestCase { - public ClientWebViewTest() { - base("ClientWebViewTest"); + public WebViewTest() { + base("Components.WebViewTest"); add_test("init_web_context", init_web_context); add_test("load_resources", load_resources); } @@ -18,7 +18,7 @@ public class ClientWebViewTest : TestCase { Application.Client.SCHEMA_ID ); config.enable_debug = true; - ClientWebView.init_web_context( + WebView.init_web_context( config, File.new_for_path(_BUILD_ROOT_DIR).get_child("src"), File.new_for_path("/tmp") // XXX use something better here @@ -27,7 +27,7 @@ public class ClientWebViewTest : TestCase { public void load_resources() throws GLib.Error { try { - ClientWebView.load_resources(GLib.File.new_for_path("/tmp")); + WebView.load_resources(GLib.File.new_for_path("/tmp")); } catch (GLib.Error err) { assert_not_reached(); } diff --git a/test/client/composer/composer-web-view-test.vala b/test/client/composer/composer-web-view-test.vala index ab935f0a..31005d5f 100644 --- a/test/client/composer/composer-web-view-test.vala +++ b/test/client/composer/composer-web-view-test.vala @@ -5,7 +5,7 @@ * (version 2.1 or later). See the COPYING file in this distribution. */ -public class Composer.WebViewTest : ClientWebViewTestCase { +public class Composer.WebViewTest : Components.WebViewTestCase { public WebViewTest() { diff --git a/test/js/client-page-state-test.vala b/test/js/components-page-state-test.vala similarity index 73% rename from test/js/client-page-state-test.vala rename to test/js/components-page-state-test.vala index 5f41f9d7..5ec75746 100644 --- a/test/js/client-page-state-test.vala +++ b/test/js/components-page-state-test.vala @@ -5,24 +5,24 @@ * (version 2.1 or later). See the COPYING file in this distribution. */ -class ClientPageStateTest : ClientWebViewTestCase { +class Components.PageStateTest : WebViewTestCase { - private class TestClientWebView : ClientWebView { + private class TestWebView : Components.WebView { - public TestClientWebView(Application.Configuration config) { + public TestWebView(Application.Configuration config) { base(config); } } - public ClientPageStateTest() { - base("ClientPageStateTest"); + public PageStateTest() { + base("Components.PageStateTest"); add_test("content_loaded", content_loaded); try { - ClientWebView.load_resources(GLib.File.new_for_path("/tmp")); + WebView.load_resources(GLib.File.new_for_path("/tmp")); } catch (GLib.Error err) { GLib.assert_not_reached(); } @@ -45,7 +45,7 @@ class ClientPageStateTest : ClientWebViewTestCase { assert(content_loaded_triggered); } - protected override ClientWebView set_up_test_view() { + protected override WebView set_up_test_view() { WebKit.UserScript test_script; test_script = new WebKit.UserScript( "var geary = new PageState()", @@ -55,7 +55,7 @@ class ClientPageStateTest : ClientWebViewTestCase { null ); - ClientWebView view = new TestClientWebView(this.config); + WebView view = new TestWebView(this.config); view.get_user_content_manager().add_script(test_script); return view; } diff --git a/test/js/composer-page-state-test.vala b/test/js/composer-page-state-test.vala index 86382fc4..1a2e2df3 100644 --- a/test/js/composer-page-state-test.vala +++ b/test/js/composer-page-state-test.vala @@ -5,7 +5,7 @@ * (version 2.1 or later). See the COPYING file in this distribution. */ -class Composer.PageStateTest : ClientWebViewTestCase { +class Composer.PageStateTest : Components.WebViewTestCase { public const string COMPLETE_BODY_TEMPLATE = """
%s


"""; diff --git a/test/js/conversation-page-state-test.vala b/test/js/conversation-page-state-test.vala index 25d91f2b..e46783e3 100644 --- a/test/js/conversation-page-state-test.vala +++ b/test/js/conversation-page-state-test.vala @@ -5,7 +5,7 @@ * (version 2.1 or later). See the COPYING file in this distribution. */ -class ConversationPageStateTest : ClientWebViewTestCase { +class ConversationPageStateTest : Components.WebViewTestCase { public ConversationPageStateTest() { base("ConversationPageStateTest"); diff --git a/test/meson.build b/test/meson.build index fe3040dd..43bc2a4c 100644 --- a/test/meson.build +++ b/test/meson.build @@ -82,9 +82,9 @@ test_client_sources = [ 'client/application/application-certificate-manager-test.vala', 'client/application/application-client-test.vala', 'client/application/application-configuration-test.vala', - 'client/components/client-web-view-test.vala', - 'client/components/client-web-view-test-case.vala', 'client/components/components-validator-test.vala', + 'client/components/components-web-view-test-case.vala', + 'client/components/components-web-view-test.vala', 'client/composer/composer-web-view-test.vala', 'client/composer/composer-widget-test.vala', 'client/util/util-avatar-test.vala', @@ -92,7 +92,7 @@ test_client_sources = [ 'client/util/util-email-test.vala', 'client/util/util-js-test.vala', - 'js/client-page-state-test.vala', + 'js/components-page-state-test.vala', 'js/composer-page-state-test.vala', 'js/conversation-page-state-test.vala', diff --git a/test/test-client.vala b/test/test-client.vala index 573aaac1..08949b4c 100644 --- a/test/test-client.vala +++ b/test/test-client.vala @@ -53,10 +53,10 @@ int main(string[] args) { client.add_suite(new Application.CertificateManagerTest().suite); client.add_suite(new Application.ClientTest().suite); client.add_suite(new Application.ConfigurationTest().suite); - client.add_suite(new ClientWebViewTest().suite); + client.add_suite(new Components.WebViewTest().suite); + client.add_suite(new Components.ValidatorTest().suite); client.add_suite(new Composer.WebViewTest().suite); client.add_suite(new Composer.WidgetTest().suite); - client.add_suite(new Components.ValidatorTest().suite); client.add_suite(new Util.Avatar.Test().suite); client.add_suite(new Util.Cache.Test().suite); client.add_suite(new Util.Email.Test().suite); @@ -64,7 +64,7 @@ int main(string[] args) { TestSuite js = new TestSuite("js"); - js.add_suite(new ClientPageStateTest().suite); + js.add_suite(new Components.PageStateTest().suite); js.add_suite(new Composer.PageStateTest().suite); js.add_suite(new ConversationPageStateTest().suite); diff --git a/ui/client-web-view-allow-remote-images.js b/ui/client-web-view-allow-remote-images.js deleted file mode 100644 index 1fb05606..00000000 --- a/ui/client-web-view-allow-remote-images.js +++ /dev/null @@ -1,11 +0,0 @@ -/* - * 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. - */ - -/** - * Enables remote image loading in a client web view. - */ -geary.allowRemoteImages = true; diff --git a/ui/client-web-view.js b/ui/components-web-view.js similarity index 98% rename from ui/client-web-view.js rename to ui/components-web-view.js index 75bdecf1..80e86d7c 100644 --- a/ui/client-web-view.js +++ b/ui/components-web-view.js @@ -6,7 +6,7 @@ */ /** - * Application logic for ClientWebView and subclasses. + * Application logic for Components.WebView and subclasses. */ let PageState = function() { @@ -14,7 +14,6 @@ let PageState = function() { }; PageState.prototype = { init: function() { - this.allowRemoteImages = false; this.isLoaded = false; this.undoEnabled = false; this.redoEnabled = false; @@ -108,7 +107,7 @@ PageState.prototype = { window.webkit.messageHandlers.contentLoaded.postMessage(null); }, loadRemoteImages: function() { - this.allowRemoteImages = true; + window._gearyAllowRemoteResourceLoads = true; let images = document.getElementsByTagName("IMG"); for (let i = 0; i < images.length; i++) { let img = images.item(i); diff --git a/ui/conversation-message.ui b/ui/conversation-message.ui index 1e520801..95560939 100644 --- a/ui/conversation-message.ui +++ b/ui/conversation-message.ui @@ -473,6 +473,7 @@ True False + slide-up True diff --git a/ui/org.gnome.Geary.gresource.xml b/ui/org.gnome.Geary.gresource.xml index e064d331..0b9e900f 100644 --- a/ui/org.gnome.Geary.gresource.xml +++ b/ui/org.gnome.Geary.gresource.xml @@ -9,8 +9,7 @@ accounts_editor_servers_pane.ui application-main-window.ui certificate_warning_dialog.glade - client-web-view.js - client-web-view-allow-remote-images.js + components-web-view.js components-attachment-pane.ui components-attachment-pane-menus.ui components-attachment-view.ui From 1ba2bd0f5ba655b38aff63d6332b0bb52c704119 Mon Sep 17 00:00:00 2001 From: Michael Gratton Date: Wed, 26 Aug 2020 15:20:12 +1000 Subject: [PATCH 02/18] Util.JS: Support converting between JSC.Value and GLib.Variant objects Add `variant_to_value` and `value_to_variant` methods, document them and add tests. --- src/client/util/util-js.vala | 159 +++++++++++++++++++++++++++++ test/client/util/util-js-test.vala | 125 +++++++++++++++++++++++ 2 files changed, 284 insertions(+) diff --git a/src/client/util/util-js.vala b/src/client/util/util-js.vala index 52c9428b..2f05a3e2 100644 --- a/src/client/util/util-js.vala +++ b/src/client/util/util-js.vala @@ -127,6 +127,165 @@ namespace Util.JS { } } + /** + * Converts a JS value to a GLib variant. + * + * Simple value objects (string, number, and Boolean values), + * arrays of these, and objects with these types as properties are + * supported. Arrays are converted to arrays of variants, and + * objects to dictionaries containing string keys and variant + * values. Null or undefined values are returned as an empty maybe + * variant type, since it is not possible to determine the actual + * type. + * + * Throws a type error if the given value's type is not supported. + */ + public inline GLib.Variant value_to_variant(JSC.Value value) + throws Error { + if (value.is_null() || value.is_undefined()) { + return new GLib.Variant.maybe(GLib.VariantType.VARIANT, null); + } + if (value.is_boolean()) { + return new GLib.Variant.boolean(value.to_boolean()); + } + if (value.is_number()) { + return new GLib.Variant.double(value.to_double()); + } + if (value.is_string()) { + return new GLib.Variant.string(value.to_string()); + } + if (value.is_array()) { + int len = to_int32(value.object_get_property("length")); + GLib.Variant[] values = new GLib.Variant[len]; + for (int i = 0; i < len; i++) { + values[i] = new GLib.Variant.variant( + value_to_variant(value.object_get_property_at_index(i)) + ); + } + return new GLib.Variant.array(GLib.VariantType.VARIANT, values); + } + if (value.is_object()) { + GLib.VariantDict dict = new GLib.VariantDict(); + string[] names = value.object_enumerate_properties(); + if (names != null) { + foreach (var name in names) { + try { + dict.insert_value( + name, + new GLib.Variant.variant( + value_to_variant( + value.object_get_property(name) + ) + ) + ); + } catch (Error.TYPE err) { + // ignored + } + } + } + return dict.end(); + } + throw new Error.TYPE("Unsupported JS type: %s", value.to_string()); + } + + /** + * Converts a GLib variant to a JS value. + * + * Simple value objects (string, number, and Boolean values), + * arrays and tuples of these, and dictionaries with string keys + * are supported. Tuples and arrays are converted to JS arrays, + * and dictionaries or tuples containing dictionary entries are + * converted to JS objects. + * + * Throws a type error if the given variant's type is not supported. + */ + public inline JSC.Value variant_to_value(JSC.Context context, + GLib.Variant variant) + throws Error.TYPE { + JSC.Value? value = null; + GLib.Variant.Class type = variant.classify(); + if (type == MAYBE) { + GLib.Variant? maybe = variant.get_maybe(); + if (maybe != null) { + value = variant_to_value(context, maybe); + } else { + value = new JSC.Value.null(context); + } + } else if (type == VARIANT) { + value = variant_to_value(context, variant.get_variant()); + } else if (type == STRING) { + value = new JSC.Value.string(context, variant.get_string()); + } else if (type == BOOLEAN) { + value = new JSC.Value.boolean(context, variant.get_boolean()); + } else if (type == DOUBLE) { + value = new JSC.Value.number(context, variant.get_double()); + } else if (type == INT64) { + value = new JSC.Value.number(context, (double) variant.get_int64()); + } else if (type == INT32) { + value = new JSC.Value.number(context, (double) variant.get_int32()); + } else if (type == INT16) { + value = new JSC.Value.number(context, (double) variant.get_int16()); + } else if (type == UINT64) { + value = new JSC.Value.number(context, (double) variant.get_uint64()); + } else if (type == UINT32) { + value = new JSC.Value.number(context, (double) variant.get_uint32()); + } else if (type == UINT16) { + value = new JSC.Value.number(context, (double) variant.get_uint16()); + } else if (type == BYTE) { + value = new JSC.Value.number(context, (double) variant.get_byte()); + } else if (type == ARRAY || + type == TUPLE) { + size_t len = variant.n_children(); + if (len == 0) { + if (type == ARRAY || + type == TUPLE) { + value = new JSC.Value.array_from_garray(context, null); + } else { + value = new JSC.Value.object(context, null, null); + } + } else { + var first = variant.get_child_value(0); + if (first.classify() == DICT_ENTRY) { + value = new JSC.Value.object(context, null, null); + for (size_t i = 0; i < len; i++) { + var entry = variant.get_child_value(i); + if (entry.classify() != DICT_ENTRY) { + throw new Error.TYPE( + "Variant mixes dict entries with others: %s", + variant.print(true) + ); + } + var key = entry.get_child_value(0); + if (key.classify() != STRING) { + throw new Error.TYPE( + "Dict entry key is not a string: %s", + entry.print(true) + ); + } + value.object_set_property( + key.get_string(), + variant_to_value(context, entry.get_child_value(1)) + ); + } + } else { + var values = new GLib.GenericArray((uint) len); + for (size_t i = 0; i < len; i++) { + values.add( + variant_to_value(context, variant.get_child_value(i)) + ); + } + value = new JSC.Value.array_from_garray(context, values); + } + } + } + if (value == null) { + throw new Error.TYPE( + "Unsupported variant type %s", variant.print(true) + ); + } + return value; + } + /** * Escapes a string so as to be safe to use as a JS string literal. * diff --git a/test/client/util/util-js-test.vala b/test/client/util/util-js-test.vala index 1fbe5276..16a01d83 100644 --- a/test/client/util/util-js-test.vala +++ b/test/client/util/util-js-test.vala @@ -7,9 +7,23 @@ public class Util.JS.Test : TestCase { + + private JSC.Context? context = null; + + public Test() { base("Util.JS.Test"); add_test("escape_string", escape_string); + add_test("to_variant", to_variant); + add_test("to_value", to_value); + } + + public override void set_up() throws GLib.Error { + this.context = new JSC.Context(); + } + + public override void tear_down() throws GLib.Error { + this.context = null; } public void escape_string() throws GLib.Error { @@ -21,4 +35,115 @@ public class Util.JS.Test : TestCase { assert(Util.JS.escape_string("something…\n") == """something…\n"""); } + + public void to_variant() throws GLib.Error { + assert_equal( + value_to_variant(new JSC.Value.null(this.context)).print(true), + "@mv nothing" + ); + assert_equal( + value_to_variant(new JSC.Value.string(this.context, "test")).print(true), + "'test'" + ); + assert_equal( + value_to_variant(new JSC.Value.number(this.context, 1.0)).print(true), + "1.0" + ); + assert_equal( + value_to_variant(new JSC.Value.boolean(this.context, true)).print(true), + "true" + ); + assert_equal( + value_to_variant(new JSC.Value.boolean(this.context, false)).print(true), + "false" + ); + + var value = new JSC.Value.array_from_garray(this.context, null); + assert_equal( + value_to_variant(value).print(true), + "@av []" + ); + var array = new GLib.GenericArray(); + array.add(new JSC.Value.string(this.context, "test")); + value = new JSC.Value.array_from_garray(this.context, array); + assert_equal( + value_to_variant(value).print(true), + "[<'test'>]" + ); + value = new JSC.Value.object(this.context, null, null); + assert_equal( + value_to_variant(value).print(true), + "@a{sv} {}" + ); + value.object_set_property( + "test", new JSC.Value.boolean(this.context, true) + ); + assert_equal( + value_to_variant(value).print(true), + "{'test': <>}" + ); + } + + public void to_value() throws GLib.Error { + var variant = new GLib.Variant.maybe(GLib.VariantType.STRING, null); + var value = variant_to_value(this.context, variant); + assert_true(value.is_null(), variant.print(true)); + + variant = new GLib.Variant.string("test"); + value = variant_to_value(this.context, variant); + assert_true(value.is_string(), variant.print(true)); + assert_equal(value.to_string(), "test", variant.print(true)); + + variant = new GLib.Variant.int32(42); + value = variant_to_value(this.context, variant); + assert_true(value.is_number(), variant.print(true)); + assert_equal(value.to_int32(), 42, variant.print(true)); + + variant = new GLib.Variant.double(42.0); + value = variant_to_value(this.context, variant); + assert_true(value.is_number(), variant.print(true)); + assert_within(value.to_double(), 42.0, 0.0000001, variant.print(true)); + + variant = new GLib.Variant.boolean(true); + value = variant_to_value(this.context, variant); + assert_true(value.is_boolean(), variant.print(true)); + assert_true(value.to_boolean(), variant.print(true)); + + variant = new GLib.Variant.boolean(false); + value = variant_to_value(this.context, variant); + assert_true(value.is_boolean(), variant.print(true)); + assert_false(value.to_boolean(), variant.print(true)); + + variant = new GLib.Variant.strv({"test"}); + value = variant_to_value(this.context, variant); + assert_true(value.is_array(), variant.print(true)); + assert_true( + value.object_get_property_at_index(0).is_string(), + variant.print(true) + ); + assert_equal( + value.object_get_property_at_index(0).to_string(), + "test", + variant.print(true) + ); + + var dict = new GLib.VariantDict(); + variant = dict.end(); + value = variant_to_value(this.context, variant); + assert_true(value.is_object(), variant.print(true)); + + dict = new GLib.VariantDict(); + dict.insert_value("test", new GLib.Variant.boolean(true)); + variant = dict.end(); + value = variant_to_value(this.context, variant); + assert_true(value.is_object(), variant.print(true)); + assert_true( + value.object_get_property("test").is_boolean(), + value.to_string() + ); + assert_true( + value.object_get_property("test").to_boolean(), + value.to_string() + ); + } } From ff565bc6efc83badbecfb48d2fbb457f4d2f681c Mon Sep 17 00:00:00 2001 From: Michael Gratton Date: Thu, 27 Aug 2020 12:12:22 +1000 Subject: [PATCH 03/18] Components.WebView: Convert to using messages for JS method invocation Use WebKitGTK UserMessage objects for invoking JS methods rather than serialising to JS strings and running those. This is possibly slightly less efficient, but removes the onus on serialising to and parsing from JS and once switched over from message handlers to UserMessage objects will be using a single uniform IPC interface for both. --- .../components/components-web-view.vala | 97 +++++++++++++++++-- src/client/composer/composer-web-view.vala | 67 ++++++------- src/client/composer/composer-widget.vala | 19 ++-- .../conversation-web-view.vala | 15 ++- src/client/util/util-js.vala | 36 ++++--- .../web-process/web-process-extension.vala | 53 ++++++++++ test/js/components-page-state-test.vala | 45 +++++++++ ui/components-web-view.js | 10 ++ 8 files changed, 268 insertions(+), 74 deletions(-) diff --git a/src/client/components/components-web-view.vala b/src/client/components/components-web-view.vala index 4bda1c11..368b6a8d 100644 --- a/src/client/components/components-web-view.vala +++ b/src/client/components/components-web-view.vala @@ -370,9 +370,7 @@ public abstract class Components.WebView : WebKit.WebView, Geary.BaseInterface { * Returns the view's content as an HTML string. */ public async string? get_html() throws Error { - return Util.JS.to_string( - yield call(Util.JS.callable("geary.getHtml"), null) - ); + return yield call_returning(Util.JS.callable("getHtml"), null); } /** @@ -410,7 +408,7 @@ public abstract class Components.WebView : WebKit.WebView, Geary.BaseInterface { * Load any remote images previously that were blocked. */ public void load_remote_images() { - this.call.begin(Util.JS.callable("geary.loadRemoteImages"), null); + this.call_void.begin(Util.JS.callable("loadRemoteImages"), null); } /** @@ -455,21 +453,100 @@ public abstract class Components.WebView : WebKit.WebView, Geary.BaseInterface { public new async void set_editable(bool enabled, Cancellable? cancellable) throws Error { - yield call( - Util.JS.callable("geary.setEditable").bool(enabled), cancellable + yield call_void( + Util.JS.callable("setEditable").bool(enabled), cancellable ); } /** * Invokes a {@link Util.JS.Callable} on this web view. + * + * This calls the given callable on the `geary` object for the + * current view, any returned value are ignored. */ - protected async JSC.Value call(Util.JS.Callable target, + protected async void call_void(Util.JS.Callable target, GLib.Cancellable? cancellable) throws GLib.Error { - WebKit.JavascriptResult result = yield run_javascript( - target.to_string(), cancellable + yield send_message_to_page( + target.to_message(), cancellable ); - return result.get_js_value(); + } + + /** + * Invokes a {@link Util.JS.Callable} on this web view. + * + * This calls the given callable on the `geary` object for the + * current view. The value returned by the call is returned by + * this method. + * + * The type parameter `T` must match the type returned by the + * call, else an error is thrown. Only simple nullable value types + * are supported for T, for more complex return types (arrays, + * dictionaries, etc) specify {@link GLib.Variant} for `T` and + * manually parse that. + */ + protected async T call_returning(Util.JS.Callable target, + GLib.Cancellable? cancellable) + throws GLib.Error { + WebKit.UserMessage? response = yield send_message_to_page( + target.to_message(), cancellable + ); + if (response == null) { + throw new Util.JS.Error.TYPE( + "Method call did not return a value: %s", target.to_string() + ); + } + GLib.Variant? param = response.parameters; + T ret_value = null; + var ret_type = typeof(T); + if (ret_type == typeof(GLib.Variant)) { + ret_value = param; + } else { + if (param != null && param.get_type().is_maybe()) { + param = param.get_maybe(); + } + if (param != null) { + // Since these replies are coming from JS via + // Util.JS.value_to_variant, they will only be one of + // string, double, bool, array or dict + var param_type = param.classify(); + if (ret_type == typeof(string) && param_type == STRING) { + ret_value = param.get_string(); + } else if (ret_type == typeof(bool) && param_type == BOOLEAN) { + ret_value = (bool?) param.get_boolean(); + } else if (ret_type == typeof(int) && param_type == DOUBLE) { + ret_value = (int?) ((int) param.get_double()); + } else if (ret_type == typeof(short) && param_type == DOUBLE) { + ret_value = (short?) ((short) param.get_double()); + } else if (ret_type == typeof(char) && param_type == DOUBLE) { + ret_value = (char?) ((char) param.get_double()); + } else if (ret_type == typeof(long) && param_type == DOUBLE) { + ret_value = (long?) ((long) param.get_double()); + } else if (ret_type == typeof(int64) && param_type == DOUBLE) { + ret_value = (int64?) ((int64) param.get_double()); + } else if (ret_type == typeof(uint) && param_type == DOUBLE) { + ret_value = (uint?) ((uint) param.get_double()); + } else if (ret_type == typeof(uchar) && param_type == DOUBLE) { + ret_value = (uchar?) ((uchar) param.get_double()); + } else if (ret_type == typeof(ushort) && param_type == DOUBLE) { + ret_value = (ushort?) ((ushort) param.get_double()); + } else if (ret_type == typeof(ulong) && param_type == DOUBLE) { + ret_value = (ulong?) ((ulong) param.get_double()); + } else if (ret_type == typeof(uint64) && param_type == DOUBLE) { + ret_value = (uint64?) ((uint64) param.get_double()); + } else if (ret_type == typeof(double) && param_type == DOUBLE) { + ret_value = (double?) param.get_double(); + } else if (ret_type == typeof(float) && param_type == DOUBLE) { + ret_value = (float?) ((float) param.get_double()); + } else { + throw new Util.JS.Error.TYPE( + "%s is not a supported type for %s", + ret_type.name(), param_type.to_string() + ); + } + } + } + return ret_value; } /** diff --git a/src/client/composer/composer-web-view.vala b/src/client/composer/composer-web-view.vala index f8ecccf6..24a2740c 100644 --- a/src/client/composer/composer-web-view.vala +++ b/src/client/composer/composer-web-view.vala @@ -202,8 +202,8 @@ public class Composer.WebView : Components.WebView { * Returns the view's content as HTML without being cleaned. */ public async string? get_html_for_draft() throws Error { - return Util.JS.to_string( - yield call(Util.JS.callable("geary.getHtml").bool(false), null) + return yield call_returning( + Util.JS.callable("getHtml").bool(false), null ); } @@ -213,8 +213,8 @@ public class Composer.WebView : Components.WebView { public void set_rich_text(bool enabled) { this.is_rich_text = enabled; if (this.is_content_loaded) { - this.call.begin( - Util.JS.callable("geary.setRichText").bool(enabled), null + this.call_void.begin( + Util.JS.callable("setRichText").bool(enabled), null ); } } @@ -223,14 +223,14 @@ public class Composer.WebView : Components.WebView { * Undoes the last edit operation. */ public void undo() { - this.call.begin(Util.JS.callable("geary.undo"), null); + this.call_void.begin(Util.JS.callable("undo"), null); } /** * Redoes the last undone edit operation. */ public void redo() { - this.call.begin(Util.JS.callable("geary.redo"), null); + this.call_void.begin(Util.JS.callable("redo"), null); } /** @@ -239,9 +239,9 @@ public class Composer.WebView : Components.WebView { * Returns an id to be used to refer to the selection in * subsequent calls. */ - public async string save_selection() throws Error { - return Util.JS.to_string( - yield call(Util.JS.callable("geary.saveSelection"), null) + public async string? save_selection() throws Error { + return yield call_returning( + Util.JS.callable("saveSelection"), null ); } @@ -249,9 +249,7 @@ public class Composer.WebView : Components.WebView { * Removes a saved selection. */ public void free_selection(string id) { - this.call.begin( - Util.JS.callable("geary.freeSelection").string(id), null - ); + this.call_void.begin(Util.JS.callable("freeSelection").string(id), null); } /** @@ -357,9 +355,9 @@ public class Composer.WebView : Components.WebView { * will be inserted wrapping the selection. */ public void insert_link(string href, string selection_id) { - this.call.begin( + this.call_void.begin( Util.JS.callable( - "geary.insertLink" + "insertLink" ).string(href).string(selection_id), null ); @@ -373,8 +371,8 @@ public class Composer.WebView : Components.WebView { * unlinked section. */ public void delete_link(string selection_id) { - this.call.begin( - Util.JS.callable("geary.deleteLink").string(selection_id), + this.call_void.begin( + Util.JS.callable("deleteLink").string(selection_id), null ); } @@ -396,23 +394,23 @@ public class Composer.WebView : Components.WebView { * Indents the line at the current text cursor location. */ public void indent_line() { - this.call.begin(Util.JS.callable("geary.indentLine"), null); + this.call_void.begin(Util.JS.callable("indentLine"), null); } public void insert_olist() { - this.call.begin(Util.JS.callable("geary.insertOrderedList"), null); + this.call_void.begin(Util.JS.callable("insertOrderedList"), null); } public void insert_ulist() { - this.call.begin(Util.JS.callable("geary.insertUnorderedList"), null); + this.call_void.begin(Util.JS.callable("insertUnorderedList"), null); } /** * Updates the signature block if it has not been deleted. */ public new void update_signature(string signature) { - this.call.begin( - Util.JS.callable("geary.updateSignature").string(signature), null + this.call_void.begin( + Util.JS.callable("updateSignature").string(signature), null ); } @@ -420,22 +418,21 @@ public class Composer.WebView : Components.WebView { * Removes the quoted message (if any) from the composer. */ public void delete_quoted_message() { - this.call.begin(Util.JS.callable("geary.deleteQuotedMessage"), null); + this.call_void.begin(Util.JS.callable("deleteQuotedMessage"), null); } /** * Determines if the editor content contains an attachment keyword. */ - public async bool contains_attachment_keywords(string keyword_spec, - string subject) { + public async bool? contains_attachment_keywords(string keyword_spec, + string subject) { try { - return Util.JS.to_bool( - yield call( - Util.JS.callable("geary.containsAttachmentKeyword") - .string(keyword_spec) - .string(subject), - null) - ); + return yield call_returning( + Util.JS.callable("containsAttachmentKeyword") + .string(keyword_spec) + .string(subject), + null + ); } catch (Error err) { debug("Error checking or attachment keywords: %s", err.message); return false; @@ -449,7 +446,7 @@ public class Composer.WebView : Components.WebView { * this. */ public async void clean_content() throws Error { - this.call.begin(Util.JS.callable("geary.cleanContent"), null); + this.call_void.begin(Util.JS.callable("cleanContent"), null); } /** @@ -459,10 +456,10 @@ public class Composer.WebView : Components.WebView { const int MAX_BREAKABLE_LEN = 72; // F=F recommended line limit const int MAX_UNBREAKABLE_LEN = 998; // SMTP line limit - string body_text = Util.JS.to_string( - yield call(Util.JS.callable("geary.getText"), null) + string? body_text = yield call_returning( + Util.JS.callable("getText"), null ); - string[] lines = body_text.split("\n"); + string[] lines = (body_text ?? "").split("\n"); GLib.StringBuilder flowed = new GLib.StringBuilder.sized(body_text.length); foreach (string line in lines) { // Strip trailing whitespace, so it doesn't look like a diff --git a/src/client/composer/composer-widget.vala b/src/client/composer/composer-widget.vala index 4c4d0caf..9148a88e 100644 --- a/src/client/composer/composer-widget.vala +++ b/src/client/composer/composer-widget.vala @@ -1450,15 +1450,16 @@ public class Composer.Widget : Gtk.EventBox, Geary.BaseInterface { confirmation = _("Send message with an empty subject?"); } else if (!has_body && !has_attachment) { confirmation = _("Send message with an empty body?"); - } else if (!has_attachment && - yield this.editor.body.contains_attachment_keywords( - string.join( - "|", - ATTACHMENT_KEYWORDS, - ATTACHMENT_KEYWORDS_LOCALISED - ), - this.subject)) { - confirmation = _("Send message without an attachment?"); + } else if (!has_attachment) { + var keywords = string.join( + "|", ATTACHMENT_KEYWORDS, ATTACHMENT_KEYWORDS_LOCALISED + ); + var contains = yield this.editor.body.contains_attachment_keywords( + keywords, this.subject + ); + if (contains != null && contains) { + confirmation = _("Send message without an attachment?"); + } } if (confirmation != null) { ConfirmationDialog dialog = new ConfirmationDialog(container.top_window, diff --git a/src/client/conversation-viewer/conversation-web-view.vala b/src/client/conversation-viewer/conversation-web-view.vala index ffa36394..d77af642 100644 --- a/src/client/conversation-viewer/conversation-web-view.vala +++ b/src/client/conversation-viewer/conversation-web-view.vala @@ -89,20 +89,18 @@ public class ConversationWebView : Components.WebView { * Returns the current selection, for prefill as find text. */ public async string? get_selection_for_find() throws Error{ - JSC.Value result = yield call( - Util.JS.callable("geary.getSelectionForFind"), null + return yield call_returning( + Util.JS.callable("getSelectionForFind"), null ); - return Util.JS.to_string(result); } /** * Returns the current selection, for quoting in a message. */ public async string? get_selection_for_quoting() throws Error { - JSC.Value result = yield call( - Util.JS.callable("geary.getSelectionForQuoting"), null + return yield call_returning( + Util.JS.callable("getSelectionForQuoting"), null ); - return Util.JS.to_string(result); } /** @@ -110,10 +108,9 @@ public class ConversationWebView : Components.WebView { */ public async int? get_anchor_target_y(string anchor_body) throws GLib.Error { - JSC.Value result = yield call( - Util.JS.callable("geary.getAnchorTargetY").string(anchor_body), null + return yield call_returning( + Util.JS.callable("getAnchorTargetY").string(anchor_body), null ); - return (int) Util.JS.to_int32(result); } /** diff --git a/src/client/util/util-js.vala b/src/client/util/util-js.vala index 2f05a3e2..d2ce9f2e 100644 --- a/src/client/util/util-js.vala +++ b/src/client/util/util-js.vala @@ -348,40 +348,54 @@ namespace Util.JS { */ public class Callable { - private string base_name; - private string[] safe_args = new string[0]; + private string name; + private GLib.Variant[] args = {}; - public Callable(string base_name) { - this.base_name = base_name; + public Callable(string name) { + this.name = name; + } + + public WebKit.UserMessage to_message() { + GLib.Variant? args = null; + if (this.args.length == 1) { + args = this.args[0]; + } else if (this.args.length > 1) { + args = new GLib.Variant.tuple(this.args); + } + return new WebKit.UserMessage(this.name, args); } public string to_string() { - return base_name + "(" + global::string.joinv(",", safe_args) + ");"; + string[] args = new string[this.args.length]; + for (int i = 0; i < args.length; i++) { + args[i] = this.args[i].print(true); + } + return this.name + "(" + global::string.joinv(",", args) + ")"; } public Callable string(string value) { - add_param("\"" + escape_string(value) + "\""); + add_param(new GLib.Variant.string(value)); return this; } public Callable double(double value) { - add_param(value.to_string()); + add_param(new GLib.Variant.double(value)); return this; } public Callable int(int value) { - add_param(value.to_string()); + add_param(new GLib.Variant.int32(value)); return this; } public Callable bool(bool value) { - add_param(value ? "true" : "false"); + add_param(new GLib.Variant.boolean(value)); return this; } - private inline void add_param(string value) { - this.safe_args += value; + private inline void add_param(GLib.Variant value) { + this.args += value; } } diff --git a/src/client/web-process/web-process-extension.vala b/src/client/web-process/web-process-extension.vala index 4bba5154..86f7f44c 100644 --- a/src/client/web-process/web-process-extension.vala +++ b/src/client/web-process/web-process-extension.vala @@ -30,6 +30,10 @@ public void webkit_web_extension_initialize_with_user_data(WebKit.WebExtension e */ public class GearyWebExtension : Object { + private const string PAGE_STATE_OBJECT_NAME = "geary"; + private const string MESSAGE_RETURN_VALUE_NAME = "__return__"; + private const string MESSAGE_EXCEPTION_NAME = "__exception__"; + private const string[] ALLOWED_SCHEMES = { "cid", "geary", "data", "blob" }; private const string REMOTE_LOAD_VAR = "_gearyAllowRemoteResourceLoads"; @@ -157,6 +161,55 @@ public class GearyWebExtension : Object { page.get_editor().selection_changed.connect(() => { selection_changed(page); }); + page.user_message_received.connect(on_page_message_received); + } + + private bool on_page_message_received(WebKit.WebPage page, + WebKit.UserMessage message) { + WebKit.Frame frame = page.get_main_frame(); + JSC.Context context = frame.get_js_context(); + JSC.Value page_state = context.get_value(PAGE_STATE_OBJECT_NAME); + + try { + JSC.Value[]? call_param = null; + GLib.Variant? message_param = message.parameters; + if (message_param != null) { + if (message_param.is_container()) { + size_t len = message_param.n_children(); + call_param = new JSC.Value[len]; + for (size_t i = 0; i < len; i++) { + call_param[i] = Util.JS.variant_to_value( + context, + message_param.get_child_value(i) + ); + } + } else { + call_param = { + Util.JS.variant_to_value(context, message_param) + }; + } + } + + JSC.Value ret = page_state.object_invoke_methodv( + message.name, call_param + ); + + // Must send a reply, even for void calls, otherwise + // WebKitGTK will complain. So return a message return + // rain hail or shine. + // https://bugs.webkit.org/show_bug.cgi?id=215880 + + message.send_reply( + new WebKit.UserMessage( + MESSAGE_RETURN_VALUE_NAME, + Util.JS.value_to_variant(ret) + ) + ); + } catch (GLib.Error err) { + debug("Failed to handle message: %s", err.message); + } + + return true; } } diff --git a/test/js/components-page-state-test.vala b/test/js/components-page-state-test.vala index 5ec75746..562c6cda 100644 --- a/test/js/components-page-state-test.vala +++ b/test/js/components-page-state-test.vala @@ -14,12 +14,24 @@ class Components.PageStateTest : WebViewTestCase { base(config); } + public new async void call_void(Util.JS.Callable callable) + throws GLib.Error { + yield base.call_void(callable, null); + } + + public new async string call_returning(Util.JS.Callable callable) + throws GLib.Error { + return yield base.call_returning(callable, null); + } + } public PageStateTest() { base("Components.PageStateTest"); add_test("content_loaded", content_loaded); + add_test("call_void", call_void); + add_test("call_returning", call_returning); try { WebView.load_resources(GLib.File.new_for_path("/tmp")); @@ -45,6 +57,30 @@ class Components.PageStateTest : WebViewTestCase { assert(content_loaded_triggered); } + public void call_void() throws GLib.Error { + load_body_fixture("OHHAI"); + var test_article = this.test_view as TestWebView; + + test_article.call_void.begin( + new Util.JS.Callable("testVoid"), this.async_completion + ); + test_article.call_void.end(this.async_result()); + assert_test_result("void"); + } + + public void call_returning() throws GLib.Error { + load_body_fixture("OHHAI"); + var test_article = this.test_view as TestWebView; + + test_article.call_returning.begin( + new Util.JS.Callable("testReturn").string("check 1-2"), + this.async_completion + ); + string ret = test_article.call_returning.end(this.async_result()); + assert_equal(ret, "check 1-2"); + assert_test_result("check 1-2"); + } + protected override WebView set_up_test_view() { WebKit.UserScript test_script; test_script = new WebKit.UserScript( @@ -60,4 +96,13 @@ class Components.PageStateTest : WebViewTestCase { return view; } + private void assert_test_result(string expected) + throws GLib.Error { + string? result = Util.JS.to_string( + run_javascript("geary.testResult") + .get_js_value() + ); + assert_equal(result, expected); + } + } diff --git a/ui/components-web-view.js b/ui/components-web-view.js index 80e86d7c..289abca0 100644 --- a/ui/components-web-view.js +++ b/ui/components-web-view.js @@ -87,6 +87,8 @@ PageState.prototype = { window.addEventListener("transitionend", function(e) { queuePreferredHeightUpdate(); }, false); // load does not bubble + + this.testResult = null; }, getPreferredHeight: function() { // Return the scroll height of the HTML element since the BODY @@ -184,5 +186,13 @@ PageState.prototype = { this.hasSelection = hasSelection; window.webkit.messageHandlers.selectionChanged.postMessage(hasSelection); } + }, + // Methods below are for unit tests. + testVoid: function() { + this.testResult = "void"; + }, + testReturn: function(value) { + this.testResult = value; + return value; } }; From c813aa5707acc5226a57dca82449dc709969d05a Mon Sep 17 00:00:00 2001 From: Michael Gratton Date: Thu, 27 Aug 2020 16:18:45 +1000 Subject: [PATCH 04/18] Components.WebView: Check for pass up exceptions when calling JS code Update web extension to check for errors when invoking page state methods and pass a message back if found. Check for this, decode and throw a vala error in the WebView if found. --- .../components/components-web-view.vala | 56 ++++++++++++++--- .../web-process/web-process-extension.vala | 39 ++++++++++-- test/js/components-page-state-test.vala | 60 +++++++++++++++++++ ui/components-web-view.js | 4 ++ 4 files changed, 146 insertions(+), 13 deletions(-) diff --git a/src/client/components/components-web-view.vala b/src/client/components/components-web-view.vala index 368b6a8d..2b373170 100644 --- a/src/client/components/components-web-view.vala +++ b/src/client/components/components-web-view.vala @@ -26,6 +26,10 @@ public abstract class Components.WebView : WebKit.WebView, Geary.BaseInterface { /** URI Scheme and delimiter for images loaded by Content-ID. */ public const string CID_URL_PREFIX = "cid:"; + // Keep these in sync with GearyWebExtension + private const string MESSAGE_RETURN_VALUE_NAME = "__return__"; + private const string MESSAGE_EXCEPTION_NAME = "__exception__"; + // WebKit message handler names private const string COMMAND_STACK_CHANGED = "commandStackChanged"; private const string CONTENT_LOADED = "contentLoaded"; @@ -467,9 +471,7 @@ public abstract class Components.WebView : WebKit.WebView, Geary.BaseInterface { protected async void call_void(Util.JS.Callable target, GLib.Cancellable? cancellable) throws GLib.Error { - yield send_message_to_page( - target.to_message(), cancellable - ); + yield call_impl(target, cancellable); } /** @@ -488,12 +490,10 @@ public abstract class Components.WebView : WebKit.WebView, Geary.BaseInterface { protected async T call_returning(Util.JS.Callable target, GLib.Cancellable? cancellable) throws GLib.Error { - WebKit.UserMessage? response = yield send_message_to_page( - target.to_message(), cancellable - ); + WebKit.UserMessage? response = yield call_impl(target, cancellable); if (response == null) { throw new Util.JS.Error.TYPE( - "Method call did not return a value: %s", target.to_string() + "Method call %s did not return a value", target.to_string() ); } GLib.Variant? param = response.parameters; @@ -612,6 +612,48 @@ public abstract class Components.WebView : WebKit.WebView, Geary.BaseInterface { "monospace-font", SettingsBindFlags.DEFAULT); } + private async WebKit.UserMessage? call_impl(Util.JS.Callable target, + GLib.Cancellable? cancellable) + throws GLib.Error { + WebKit.UserMessage? response = yield send_message_to_page( + target.to_message(), cancellable + ); + if (response != null) { + var response_name = response.name; + if (response_name == MESSAGE_EXCEPTION_NAME) { + var exception = new GLib.VariantDict(response.parameters); + var name = exception.lookup_value("name", GLib.VariantType.STRING) as string; + var message = exception.lookup_value("message", GLib.VariantType.STRING) as string; + var backtrace = exception.lookup_value("backtrace_string", GLib.VariantType.STRING) as string; + var source = exception.lookup_value("source_uri", GLib.VariantType.STRING) as string; + var line = exception.lookup_value("line_number", GLib.VariantType.UINT32); + var column = exception.lookup_value("column_number", GLib.VariantType.UINT32); + + var log_message = "Method call %s raised %s exception at %s:%d:%d: %s".printf( + target.to_string(), + name ?? "unknown", + source ?? "unknown", + (line != null ? (int) line.get_uint32() : -1), + (column != null ? (int) column.get_uint32() : -1), + message ?? "unknown" + ); + debug(log_message); + if (backtrace != null) { + debug(backtrace); + } + + throw new Util.JS.Error.EXCEPTION(log_message); + } else if (response_name != MESSAGE_RETURN_VALUE_NAME) { + throw new Util.JS.Error.TYPE( + "Method call %s returned unknown name: %s", + target.to_string(), + response_name + ); + } + } + return response; + } + private void handle_cid_request(WebKit.URISchemeRequest request) { if (!handle_internal_response(request)) { request.finish_error(new FileError.NOENT("Unknown CID")); diff --git a/src/client/web-process/web-process-extension.vala b/src/client/web-process/web-process-extension.vala index 86f7f44c..7aa6dd3c 100644 --- a/src/client/web-process/web-process-extension.vala +++ b/src/client/web-process/web-process-extension.vala @@ -31,6 +31,8 @@ public void webkit_web_extension_initialize_with_user_data(WebKit.WebExtension e public class GearyWebExtension : Object { private const string PAGE_STATE_OBJECT_NAME = "geary"; + + // Keep these in sync with Components.WebView private const string MESSAGE_RETURN_VALUE_NAME = "__return__"; private const string MESSAGE_EXCEPTION_NAME = "__exception__"; @@ -199,12 +201,37 @@ public class GearyWebExtension : Object { // rain hail or shine. // https://bugs.webkit.org/show_bug.cgi?id=215880 - message.send_reply( - new WebKit.UserMessage( - MESSAGE_RETURN_VALUE_NAME, - Util.JS.value_to_variant(ret) - ) - ); + JSC.Exception? thrown = context.get_exception(); + if (thrown != null) { + var detail = new GLib.VariantDict(); + if (thrown.get_message() != null) { + detail.insert_value("name", new GLib.Variant.string(thrown.get_name())); + } + if (thrown.get_message() != null) { + detail.insert_value("message", new GLib.Variant.string(thrown.get_message())); + } + if (thrown.get_backtrace_string() != null) { + detail.insert_value("backtrace_string", new GLib.Variant.string(thrown.get_backtrace_string())); + } + if (thrown.get_source_uri() != null) { + detail.insert_value("source_uri", new GLib.Variant.string(thrown.get_source_uri())); + } + detail.insert_value("line_number", new GLib.Variant.uint32(thrown.get_line_number())); + detail.insert_value("column_number", new GLib.Variant.uint32(thrown.get_column_number())); + message.send_reply( + new WebKit.UserMessage( + MESSAGE_EXCEPTION_NAME, + detail.end() + ) + ); + } else { + message.send_reply( + new WebKit.UserMessage( + MESSAGE_RETURN_VALUE_NAME, + Util.JS.value_to_variant(ret) + ) + ); + } } catch (GLib.Error err) { debug("Failed to handle message: %s", err.message); } diff --git a/test/js/components-page-state-test.vala b/test/js/components-page-state-test.vala index 562c6cda..bf952416 100644 --- a/test/js/components-page-state-test.vala +++ b/test/js/components-page-state-test.vala @@ -31,7 +31,9 @@ class Components.PageStateTest : WebViewTestCase { base("Components.PageStateTest"); add_test("content_loaded", content_loaded); add_test("call_void", call_void); + add_test("call_void_throws", call_void_throws); add_test("call_returning", call_returning); + add_test("call_returning_throws", call_returning_throws); try { WebView.load_resources(GLib.File.new_for_path("/tmp")); @@ -68,6 +70,35 @@ class Components.PageStateTest : WebViewTestCase { assert_test_result("void"); } + public void call_void_throws() throws GLib.Error { + load_body_fixture("OHHAI"); + var test_article = this.test_view as TestWebView; + + try { + test_article.call_void.begin( + new Util.JS.Callable("testThrow").string("void message"), + this.async_completion + ); + test_article.call_void.end(this.async_result()); + assert_not_reached(); + } catch (Util.JS.Error.EXCEPTION err) { + assert_string( + err.message + ).contains( + "testThrow" + // WebKitGTK doesn't actually pass any details through: + // https://bugs.webkit.org/show_bug.cgi?id=215877 + // ).contains( + // "Error" + // ).contains( + // "void message" + // ).contains( + // "components-web-view.js" + ); + assert_test_result("void message"); + } + } + public void call_returning() throws GLib.Error { load_body_fixture("OHHAI"); var test_article = this.test_view as TestWebView; @@ -81,6 +112,35 @@ class Components.PageStateTest : WebViewTestCase { assert_test_result("check 1-2"); } + public void call_returning_throws() throws GLib.Error { + load_body_fixture("OHHAI"); + var test_article = this.test_view as TestWebView; + + try { + test_article.call_returning.begin( + new Util.JS.Callable("testThrow").string("return message"), + this.async_completion + ); + test_article.call_returning.end(this.async_result()); + assert_not_reached(); + } catch (Util.JS.Error.EXCEPTION err) { + assert_string( + err.message + ).contains( + "testThrow" + // WebKitGTK doesn't actually pass any details through: + // https://bugs.webkit.org/show_bug.cgi?id=215877 + // ).contains( + // "Error" + // ).contains( + // "return message" + // ).contains( + // "components-web-view.js" + ); + assert_test_result("return message"); + } + } + protected override WebView set_up_test_view() { WebKit.UserScript test_script; test_script = new WebKit.UserScript( diff --git a/ui/components-web-view.js b/ui/components-web-view.js index 289abca0..0f932a19 100644 --- a/ui/components-web-view.js +++ b/ui/components-web-view.js @@ -194,5 +194,9 @@ PageState.prototype = { testReturn: function(value) { this.testResult = value; return value; + }, + testThrow: function(value) { + this.testResult = value; + throw this.testResult; } }; From db69807836cb2485af0941a83f57451c034b21a0 Mon Sep 17 00:00:00 2001 From: Michael Gratton Date: Fri, 28 Aug 2020 09:44:46 +1000 Subject: [PATCH 05/18] GearyWebExtension: Add factory method for error user messages --- .../web-process/web-process-extension.vala | 56 +++++++++++++------ 1 file changed, 38 insertions(+), 18 deletions(-) diff --git a/src/client/web-process/web-process-extension.vala b/src/client/web-process/web-process-extension.vala index 7aa6dd3c..89d9a1e3 100644 --- a/src/client/web-process/web-process-extension.vala +++ b/src/client/web-process/web-process-extension.vala @@ -145,6 +145,37 @@ public class GearyWebExtension : Object { return ret; } + private WebKit.UserMessage to_exception_message(string? name, + string? message, + string? backtrace = null, + string? source = null, + int line_number = -1, + int column_number = -1) { + var detail = new GLib.VariantDict(); + if (name != null) { + detail.insert_value("name", new GLib.Variant.string(name)); + } + if (message != null) { + detail.insert_value("message", new GLib.Variant.string(message)); + } + if (backtrace != null) { + detail.insert_value("backtrace", new GLib.Variant.string(backtrace)); + } + if (source != null) { + detail.insert_value("source", new GLib.Variant.string(source)); + } + if (line_number > 0) { + detail.insert_value("line_number", new GLib.Variant.uint32(line_number)); + } + if (column_number > 0) { + detail.insert_value("column_number", new GLib.Variant.uint32(column_number)); + } + return new WebKit.UserMessage( + MESSAGE_EXCEPTION_NAME, + detail.end() + ); + } + private void on_page_created(WebKit.WebExtension extension, WebKit.WebPage page) { WebKit.Frame frame = page.get_main_frame(); @@ -203,25 +234,14 @@ public class GearyWebExtension : Object { JSC.Exception? thrown = context.get_exception(); if (thrown != null) { - var detail = new GLib.VariantDict(); - if (thrown.get_message() != null) { - detail.insert_value("name", new GLib.Variant.string(thrown.get_name())); - } - if (thrown.get_message() != null) { - detail.insert_value("message", new GLib.Variant.string(thrown.get_message())); - } - if (thrown.get_backtrace_string() != null) { - detail.insert_value("backtrace_string", new GLib.Variant.string(thrown.get_backtrace_string())); - } - if (thrown.get_source_uri() != null) { - detail.insert_value("source_uri", new GLib.Variant.string(thrown.get_source_uri())); - } - detail.insert_value("line_number", new GLib.Variant.uint32(thrown.get_line_number())); - detail.insert_value("column_number", new GLib.Variant.uint32(thrown.get_column_number())); message.send_reply( - new WebKit.UserMessage( - MESSAGE_EXCEPTION_NAME, - detail.end() + to_exception_message( + thrown.get_name(), + thrown.get_message(), + thrown.get_backtrace_string(), + thrown.get_source_uri(), + (int) thrown.get_line_number(), + (int) thrown.get_column_number() ) ); } else { From 6162785d997fcfa4efaf6ec83670b2fab8cca6bd Mon Sep 17 00:00:00 2001 From: Michael Gratton Date: Fri, 28 Aug 2020 09:49:46 +1000 Subject: [PATCH 06/18] GearyWebExtension: Add support for sending messages from JS to client Define a vala-backed JS class in the extension and make that available to pages when they are registered. Add some helper JS to PageState for defining message sending functions. Listen for these in Components.WebView and dispatch to the registered callback for it. --- .../components/components-web-view.vala | 55 ++++++++++++++++ .../web-process/web-process-extension.vala | 63 +++++++++++++++++++ ui/components-web-view.js | 9 +++ 3 files changed, 127 insertions(+) diff --git a/src/client/components/components-web-view.vala b/src/client/components/components-web-view.vala index 2b373170..c746c441 100644 --- a/src/client/components/components-web-view.vala +++ b/src/client/components/components-web-view.vala @@ -198,6 +198,24 @@ public abstract class Components.WebView : WebKit.WebView, Geary.BaseInterface { /** Delegate for UserContentManager message callbacks. */ public delegate void JavaScriptMessageHandler(WebKit.JavascriptResult js_result); + /** + * Delegate for message handler callbacks. + * + * @see register_message_callback + */ + protected delegate void MessageCallback(GLib.Variant? parameters); + + // Work around for not being able to put delegates in a Gee collection. + private class MessageCallable { + + public unowned MessageCallback handler; + + public MessageCallable(MessageCallback handler) { + this.handler = handler; + } + + } + /** * Determines if the view's content has been fully loaded. * @@ -263,6 +281,8 @@ public abstract class Components.WebView : WebKit.WebView, Geary.BaseInterface { private Gee.List registered_message_handlers = new Gee.LinkedList(); + private Gee.Map message_handlers = + new Gee.HashMap(); private double webkit_reported_height = 0; @@ -359,6 +379,7 @@ public abstract class Components.WebView : WebKit.WebView, Geary.BaseInterface { this.user_content_manager.disconnect(id); } this.registered_message_handlers.clear(); + this.message_handlers.clear(); base.destroy(); } @@ -568,6 +589,14 @@ public abstract class Components.WebView : WebKit.WebView, Geary.BaseInterface { } } + /** + * Registers a callback for a specific WebKit user message. + */ + protected void register_message_callback(string name, + MessageCallback handler) { + this.message_handlers.set(name, new MessageCallable(handler)); + } + private void init(Application.Configuration config) { // XXX get the allow prefix from the extension somehow @@ -595,6 +624,8 @@ public abstract class Components.WebView : WebKit.WebView, Geary.BaseInterface { SELECTION_CHANGED, on_selection_changed ); + this.user_message_received.connect(this.on_message_received); + // Manage zoom level, ensure it's sane config.bind(Application.Configuration.CONVERSATION_VIEWER_ZOOM_KEY, this, "zoom_level"); if (this.zoom_level < ZOOM_MIN) { @@ -803,6 +834,30 @@ public abstract class Components.WebView : WebKit.WebView, Geary.BaseInterface { } } + private bool on_message_received(WebKit.UserMessage message) { + if (message.name == MESSAGE_EXCEPTION_NAME) { + var detail = new GLib.VariantDict(message.parameters); + var name = detail.lookup_value("name", GLib.VariantType.STRING) as string; + var log_message = detail.lookup_value("message", GLib.VariantType.STRING) as string; + warning( + "Error sending message from JS: %s: %s", + name ?? "unknown", + log_message ?? "unknown" + ); + } else if (this.message_handlers.has_key(message.name)) { + debug( + "Message received: %s(%s)", + message.name, + message.parameters != null ? message.parameters.print(true) : "" + ); + MessageCallable callback = this.message_handlers.get(message.name); + callback.handler(message.parameters); + } else { + warning("Message with unknown handler received: %s", message.name); + } + return true; + } + } // XXX this needs to be moved into the libsoup bindings diff --git a/src/client/web-process/web-process-extension.vala b/src/client/web-process/web-process-extension.vala index 89d9a1e3..31f2b0f0 100644 --- a/src/client/web-process/web-process-extension.vala +++ b/src/client/web-process/web-process-extension.vala @@ -38,6 +38,8 @@ public class GearyWebExtension : Object { private const string[] ALLOWED_SCHEMES = { "cid", "geary", "data", "blob" }; + private const string EXTENSION_CLASS_VAR = "_GearyWebExtension"; + private const string EXTENSION_CLASS_SEND = "send"; private const string REMOTE_LOAD_VAR = "_gearyAllowRemoteResourceLoads"; private WebKit.WebExtension extension; @@ -180,6 +182,25 @@ public class GearyWebExtension : Object { WebKit.WebPage page) { WebKit.Frame frame = page.get_main_frame(); JSC.Context context = frame.get_js_context(); + + var extension_class = context.register_class( + this.get_type().name(), + null, + null, + null + ); + extension_class.add_method( + EXTENSION_CLASS_SEND, + (instance, values) => { + return this.on_page_send_message(page, values); + }, + GLib.Type.NONE + ); + context.set_value( + EXTENSION_CLASS_VAR, + new JSC.Value.object(context, extension_class, extension_class) + ); + context.set_value( REMOTE_LOAD_VAR, new JSC.Value.boolean(context, false) @@ -259,4 +280,46 @@ public class GearyWebExtension : Object { return true; } + private bool on_page_send_message(WebKit.WebPage page, + GLib.GenericArray args) { + WebKit.UserMessage? message = null; + if (args.length > 0) { + var name = args.get(0).to_string(); + GLib.Variant? parameters = null; + if (args.length > 1) { + JSC.Value param_value = args.get(1); + try { + int len = Util.JS.to_int32( + param_value.object_get_property("length") + ); + if (len == 1) { + parameters = Util.JS.value_to_variant( + param_value.object_get_property_at_index(0) + ); + } else if (len > 1) { + parameters = Util.JS.value_to_variant(param_value); + } + } catch (Util.JS.Error err) { + message = to_exception_message( + this.get_type().name(), err.message + ); + } + } + if (message == null) { + message = new WebKit.UserMessage(name, parameters); + } + } + if (message == null) { + var log_message = "Not enough parameters for JS call to %s.%s()".printf( + EXTENSION_CLASS_VAR, + EXTENSION_CLASS_SEND + ); + debug(log_message); + message = to_exception_message(this.get_type().name(), log_message); + } + + page.send_message_to_view.begin(message, null); + return true; + } + } diff --git a/ui/components-web-view.js b/ui/components-web-view.js index 0f932a19..35e82dfc 100644 --- a/ui/components-web-view.js +++ b/ui/components-web-view.js @@ -200,3 +200,12 @@ PageState.prototype = { throw this.testResult; } }; + +let MessageSender = function(name) { + return function() { + // Since typeof(arguments) == 'object', convert to an array so + // that Components.WebView.MessageCallback callbacks get + // arrays or tuples rather than dicts as arguments + _GearyWebExtension.send(name, Array.from(arguments)); + }; +}; From 89453931bf6743049644274fc730a10e7bd2a53d Mon Sep 17 00:00:00 2001 From: Michael Gratton Date: Fri, 28 Aug 2020 11:20:27 +1000 Subject: [PATCH 07/18] Util.Js: Improve JSC Value to GLib.Variant conversion Stop needlessly wrapping object members and array elements in variant variants. Don't wrap object values in variants since the code is already using vardicts for these. Return a variant array if a JS array contains values of all the same type and don't wrap these in variants, else return a tuple, which don't need to be wrapped either. --- src/client/util/util-js.vala | 160 +++++++++++++++++++++-------- test/client/util/util-js-test.vala | 28 ++++- 2 files changed, 143 insertions(+), 45 deletions(-) diff --git a/src/client/util/util-js.vala b/src/client/util/util-js.vala index d2ce9f2e..095f9da4 100644 --- a/src/client/util/util-js.vala +++ b/src/client/util/util-js.vala @@ -1,5 +1,5 @@ /* - * Copyright 2017,2019 Michael James Gratton + * Copyright © 2017-2020 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. @@ -25,6 +25,64 @@ namespace Util.JS { TYPE } + /** Supported types of JSC values. */ + public enum JscType { + + /** Specifies an unsupported value type. */ + UNKNOWN, + + /** Specifies a JavaScript `undefined` value. */ + UNDEFINED, + + /** Specifies a JavaScript `null` value. */ + NULL, + FUNCTION, + STRING, + NUMBER, + BOOLEAN, + ARRAY, + CONSTRUCTOR, + OBJECT; + + /** + * Determines the type of a JSC value. + * + * Returns the type of the given value, or {@link UNKNOWN} if + * it could not be determined. + */ + public static JscType to_type(JSC.Value value) { + if (value.is_undefined()) { + return UNDEFINED; + } + if (value.is_null()) { + return NULL; + } + if (value.is_string()) { + return STRING; + } + if (value.is_number()) { + return NUMBER; + } + if (value.is_boolean()) { + return BOOLEAN; + } + if (value.is_array()) { + return ARRAY; + } + if (value.is_object()) { + return OBJECT; + } + if (value.is_function()) { + return FUNCTION; + } + if (value.is_constructor()) { + return CONSTRUCTOR; + } + return UNKNOWN; + } + + } + /** * Returns a JSC Value as a bool. * @@ -132,60 +190,80 @@ namespace Util.JS { * * Simple value objects (string, number, and Boolean values), * arrays of these, and objects with these types as properties are - * supported. Arrays are converted to arrays of variants, and - * objects to dictionaries containing string keys and variant - * values. Null or undefined values are returned as an empty maybe - * variant type, since it is not possible to determine the actual - * type. + * supported. Arrays containing objects of the same type are + * converted to arrays, otherwise they are converted to tuples, + * empty arrays are converted to the unit tuple, and objects are + * converted to vardict containing property names as keys and + * values. Null and undefined values are returned as an empty + * maybe variant type, since it is not possible to determine the + * actual type. * * Throws a type error if the given value's type is not supported. */ public inline GLib.Variant value_to_variant(JSC.Value value) throws Error { - if (value.is_null() || value.is_undefined()) { - return new GLib.Variant.maybe(GLib.VariantType.VARIANT, null); - } - if (value.is_boolean()) { - return new GLib.Variant.boolean(value.to_boolean()); - } - if (value.is_number()) { - return new GLib.Variant.double(value.to_double()); - } - if (value.is_string()) { - return new GLib.Variant.string(value.to_string()); - } - if (value.is_array()) { + GLib.Variant? variant = null; + switch (JscType.to_type(value)) { + case UNDEFINED: + case NULL: + variant = new GLib.Variant.maybe(GLib.VariantType.VARIANT, null); + break; + + case STRING: + variant = new GLib.Variant.string(value.to_string()); + break; + + case NUMBER: + variant = new GLib.Variant.double(value.to_double()); + break; + + case BOOLEAN: + variant = new GLib.Variant.boolean(value.to_boolean()); + break; + + case ARRAY: int len = to_int32(value.object_get_property("length")); - GLib.Variant[] values = new GLib.Variant[len]; - for (int i = 0; i < len; i++) { - values[i] = new GLib.Variant.variant( - value_to_variant(value.object_get_property_at_index(i)) - ); + if (len == 0) { + variant = new GLib.Variant.tuple({}); + } else { + JSC.Value element = value.object_get_property_at_index(0); + var first_type = JscType.to_type(element); + var all_same_type = true; + var values = new GLib.Variant[len]; + values[0] = value_to_variant(element); + for (int i = 1; i < len; i++) { + element = value.object_get_property_at_index(i); + values[i] = value_to_variant(element); + all_same_type &= (first_type == JscType.to_type(element)); + } + if (!all_same_type) { + variant = new GLib.Variant.tuple(values); + } else { + variant = new GLib.Variant.array( + values[0].get_type(), values + ); + } } - return new GLib.Variant.array(GLib.VariantType.VARIANT, values); - } - if (value.is_object()) { + break; + + case OBJECT: GLib.VariantDict dict = new GLib.VariantDict(); string[] names = value.object_enumerate_properties(); if (names != null) { foreach (var name in names) { - try { - dict.insert_value( - name, - new GLib.Variant.variant( - value_to_variant( - value.object_get_property(name) - ) - ) - ); - } catch (Error.TYPE err) { - // ignored - } + dict.insert_value( + name, + value_to_variant(value.object_get_property(name)) + ); } } - return dict.end(); + variant = dict.end(); + break; + + default: + throw new Error.TYPE("Unsupported JS type: %s", value.to_string()); } - throw new Error.TYPE("Unsupported JS type: %s", value.to_string()); + return variant; } /** diff --git a/test/client/util/util-js-test.vala b/test/client/util/util-js-test.vala index 16a01d83..f1da043d 100644 --- a/test/client/util/util-js-test.vala +++ b/test/client/util/util-js-test.vala @@ -1,5 +1,5 @@ /* - * Copyright 2017 Michael Gratton + * Copyright © 2017-2020 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. @@ -61,15 +61,35 @@ public class Util.JS.Test : TestCase { var value = new JSC.Value.array_from_garray(this.context, null); assert_equal( value_to_variant(value).print(true), - "@av []" + "()" ); + var array = new GLib.GenericArray(); array.add(new JSC.Value.string(this.context, "test")); value = new JSC.Value.array_from_garray(this.context, array); assert_equal( value_to_variant(value).print(true), - "[<'test'>]" + "['test']" ); + + array = new GLib.GenericArray(); + array.add(new JSC.Value.string(this.context, "test1")); + array.add(new JSC.Value.string(this.context, "test2")); + value = new JSC.Value.array_from_garray(this.context, array); + assert_equal( + value_to_variant(value).print(true), + "['test1', 'test2']" + ); + + array = new GLib.GenericArray(); + array.add(new JSC.Value.string(this.context, "test")); + array.add(new JSC.Value.boolean(this.context, true)); + value = new JSC.Value.array_from_garray(this.context, array); + assert_equal( + value_to_variant(value).print(true), + "('test', true)" + ); + value = new JSC.Value.object(this.context, null, null); assert_equal( value_to_variant(value).print(true), @@ -80,7 +100,7 @@ public class Util.JS.Test : TestCase { ); assert_equal( value_to_variant(value).print(true), - "{'test': <>}" + "{'test': }" ); } From fb96676fbd4ab7f413071f18cbf444cd7a953b77 Mon Sep 17 00:00:00 2001 From: Michael Gratton Date: Fri, 28 Aug 2020 11:25:28 +1000 Subject: [PATCH 08/18] =?UTF-8?q?Components.WebView:=20Convert=20to=20usin?= =?UTF-8?q?g=20messages=20for=20JS=20=E2=86=92=20client=20comms?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../components/components-web-view.vala | 79 +++++++++---------- ui/components-web-view.js | 25 +++--- 2 files changed, 53 insertions(+), 51 deletions(-) diff --git a/src/client/components/components-web-view.vala b/src/client/components/components-web-view.vala index c746c441..a5cdfe33 100644 --- a/src/client/components/components-web-view.vala +++ b/src/client/components/components-web-view.vala @@ -1,6 +1,6 @@ /* - * Copyright 2016 Software Freedom Conservancy Inc. - * Copyright 2016-2019 Michael Gratton + * Copyright © 2016 Software Freedom Conservancy Inc. + * Copyright © 2016-2020 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. @@ -31,12 +31,12 @@ public abstract class Components.WebView : WebKit.WebView, Geary.BaseInterface { private const string MESSAGE_EXCEPTION_NAME = "__exception__"; // WebKit message handler names - private const string COMMAND_STACK_CHANGED = "commandStackChanged"; - private const string CONTENT_LOADED = "contentLoaded"; - private const string DOCUMENT_MODIFIED = "documentModified"; - private const string PREFERRED_HEIGHT_CHANGED = "preferredHeightChanged"; - private const string REMOTE_IMAGE_LOAD_BLOCKED = "remoteImageLoadBlocked"; - private const string SELECTION_CHANGED = "selectionChanged"; + private const string COMMAND_STACK_CHANGED = "command_stack_changed"; + private const string CONTENT_LOADED = "content_loaded"; + private const string DOCUMENT_MODIFIED = "document_modified"; + private const string PREFERRED_HEIGHT_CHANGED = "preferred_height_changed"; + private const string REMOTE_IMAGE_LOAD_BLOCKED = "remote_image_load_blocked"; + private const string SELECTION_CHANGED = "selection_changed"; private const double ZOOM_DEFAULT = 1.0; private const double ZOOM_FACTOR = 0.1; @@ -605,22 +605,22 @@ public abstract class Components.WebView : WebKit.WebView, Geary.BaseInterface { warning("Web process crashed: %s", reason.to_string()); }); - register_message_handler( + register_message_callback( COMMAND_STACK_CHANGED, on_command_stack_changed ); - register_message_handler( + register_message_callback( CONTENT_LOADED, on_content_loaded ); - register_message_handler( + register_message_callback( DOCUMENT_MODIFIED, on_document_modified ); - register_message_handler( + register_message_callback( PREFERRED_HEIGHT_CHANGED, on_preferred_height_changed ); - register_message_handler( + register_message_callback( REMOTE_IMAGE_LOAD_BLOCKED, on_remote_image_load_blocked ); - register_message_handler( + register_message_callback( SELECTION_CHANGED, on_selection_changed ); @@ -783,12 +783,12 @@ public abstract class Components.WebView : WebKit.WebView, Geary.BaseInterface { return false; } - private void on_preferred_height_changed(WebKit.JavascriptResult result) { + private void on_preferred_height_changed(GLib.Variant? parameters) { double height = this.webkit_reported_height; - try { - height = Util.JS.to_double(result.get_js_value()); - } catch (Util.JS.Error err) { - debug("Could not get preferred height: %s", err.message); + if (parameters != null && parameters.classify() == DOUBLE) { + height = parameters.get_double(); + } else { + warning("Could not get JS preferred height"); } if (this.webkit_reported_height != height) { @@ -797,40 +797,39 @@ public abstract class Components.WebView : WebKit.WebView, Geary.BaseInterface { } } - private void on_command_stack_changed(WebKit.JavascriptResult result) { - try { - string[] values = - Util.JS.to_string(result.get_js_value()).split(","); - command_stack_changed(values[0] == "true", values[1] == "true"); - } catch (Util.JS.Error err) { - debug("Could not get command stack state: %s", err.message); + private void on_command_stack_changed(GLib.Variant? parameters) { + if (parameters != null && + parameters.is_container() && + parameters.n_children() == 2) { + GLib.Variant can_undo = parameters.get_child_value(0); + GLib.Variant can_redo = parameters.get_child_value(1); + command_stack_changed( + can_undo.classify() == BOOLEAN && can_undo.get_boolean(), + can_redo.classify() == BOOLEAN && can_redo.get_boolean() + ); + } else { + warning("Could not get JS command stack state"); } } - private void on_document_modified(WebKit.JavascriptResult result) { + private void on_document_modified(GLib.Variant? parameters) { document_modified(); } - private void on_remote_image_load_blocked(WebKit.JavascriptResult result) { + private void on_remote_image_load_blocked(GLib.Variant? parameters) { remote_image_load_blocked(); } - private void on_content_loaded(WebKit.JavascriptResult result) { + private void on_content_loaded(GLib.Variant? parameters) { this.is_content_loaded = true; content_loaded(); } - private void on_selection_changed(WebKit.JavascriptResult result) { - try { - bool has_selection = Util.JS.to_bool(result.get_js_value()); - // Avoid firing multiple notifies if the value hasn't - // changed - if (this.has_selection != has_selection) { - this.has_selection = has_selection; - } - selection_changed(has_selection); - } catch (Util.JS.Error err) { - debug("Could not get selection content: %s", err.message); + private void on_selection_changed(GLib.Variant? parameters) { + if (parameters != null && parameters.classify() == BOOLEAN) { + selection_changed(parameters.get_boolean()); + } else { + warning("Could not get JS selection value"); } } diff --git a/ui/components-web-view.js b/ui/components-web-view.js index 35e82dfc..29b6acd5 100644 --- a/ui/components-web-view.js +++ b/ui/components-web-view.js @@ -1,5 +1,5 @@ /* - * Copyright 2016 Michael Gratton + * Copyright © 2016-2020 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. @@ -20,6 +20,13 @@ PageState.prototype = { this.hasSelection = false; this.lastPreferredHeight = 0; + this._selectionChanged = MessageSender("selection_changed"); + this._contentLoaded = MessageSender("content_loaded"); + this._remoteImageLoadBlocked = MessageSender("remote_image_load_blocked"); + this._preferredHeightChanged = MessageSender("preferred_height_changed"); + this._commandStackChanged = MessageSender("command_stack_changed"); + this._documentModified = MessageSender("document_modified"); + let state = this; // Set up an observer to keep track of modifications made to @@ -106,7 +113,7 @@ PageState.prototype = { // be vaguegly correct when notifying of the HTML load // completing. this.updatePreferredHeight(); - window.webkit.messageHandlers.contentLoaded.postMessage(null); + this._contentLoaded(); }, loadRemoteImages: function() { window._gearyAllowRemoteResourceLoads = true; @@ -142,7 +149,7 @@ PageState.prototype = { this.bodyObserver.disconnect(); }, remoteImageLoadBlocked: function() { - window.webkit.messageHandlers.remoteImageLoadBlocked.postMessage(null); + this._remoteImageLoadBlocked(); }, /** * Sends "preferredHeightChanged" message if it has changed. @@ -160,9 +167,7 @@ PageState.prototype = { // shrink again, leading to visual flicker. if (this.isLoaded && height > 0 && height != this.lastPreferredHeight) { this.lastPreferredHeight = height; - window.webkit.messageHandlers.preferredHeightChanged.postMessage( - height - ); + this._preferredHeightChanged(height); } }, checkCommandStack: function() { @@ -172,19 +177,17 @@ PageState.prototype = { if (canUndo != this.undoEnabled || canRedo != this.redoEnabled) { this.undoEnabled = canUndo; this.redoEnabled = canRedo; - window.webkit.messageHandlers.commandStackChanged.postMessage( - this.undoEnabled + "," + this.redoEnabled - ); + this._commandStackChanged(this.undoEnabled, this.redoEnabled); } }, documentModified: function(element) { - window.webkit.messageHandlers.documentModified.postMessage(null); + this._documentModified(); }, selectionChanged: function() { let hasSelection = !window.getSelection().isCollapsed; if (this.hasSelection != hasSelection) { this.hasSelection = hasSelection; - window.webkit.messageHandlers.selectionChanged.postMessage(hasSelection); + this._selectionChanged(hasSelection); } }, // Methods below are for unit tests. From 3655f4896f2b33f47a94c8e7dc4f9623ba42fc2e Mon Sep 17 00:00:00 2001 From: Michael Gratton Date: Fri, 28 Aug 2020 11:38:06 +1000 Subject: [PATCH 09/18] =?UTF-8?q?Composer.WebView:=20Convert=20to=20using?= =?UTF-8?q?=20messages=20for=20JS=20=E2=86=92=20client=20comms?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/client/composer/composer-web-view.vala | 69 ++++++++++------------ ui/composer-web-view.js | 18 +++--- 2 files changed, 42 insertions(+), 45 deletions(-) diff --git a/src/client/composer/composer-web-view.vala b/src/client/composer/composer-web-view.vala index 24a2740c..57b7e1e4 100644 --- a/src/client/composer/composer-web-view.vala +++ b/src/client/composer/composer-web-view.vala @@ -33,8 +33,8 @@ public class Composer.WebView : Components.WebView { private const string SPACER = "

"; // WebKit message handler names - private const string CURSOR_CONTEXT_CHANGED = "cursorContextChanged"; - private const string DRAG_DROP_RECEIVED = "dragDropReceived"; + private const string CURSOR_CONTEXT_CHANGED = "cursor_context_changed"; + private const string DRAG_DROP_RECEIVED = "drag_drop_received"; /** * Encapsulates editing-related state for a specific DOM node. @@ -152,8 +152,8 @@ public class Composer.WebView : Components.WebView { this.user_content_manager.add_style_sheet(WebView.app_style); this.user_content_manager.add_script(WebView.app_script); - register_message_handler(CURSOR_CONTEXT_CHANGED, on_cursor_context_changed); - register_message_handler(DRAG_DROP_RECEIVED, on_drag_drop_received); + register_message_callback(CURSOR_CONTEXT_CHANGED, on_cursor_context_changed); + register_message_callback(DRAG_DROP_RECEIVED, on_drag_drop_received); // XXX this is a bit of a hack given the docs for is_empty, // above @@ -530,50 +530,43 @@ public class Composer.WebView : Components.WebView { return ret; } - private void on_cursor_context_changed(WebKit.JavascriptResult result) { - try { - cursor_context_changed( - new EditContext(Util.JS.to_string(result.get_js_value())) - ); - } catch (Util.JS.Error err) { - debug("Could not get text cursor style: %s", err.message); + private void on_cursor_context_changed(GLib.Variant? parameters) { + if (parameters != null && parameters.classify() == STRING) { + cursor_context_changed(new EditContext(parameters as string)); + } else { + warning("Could not get text cursor style"); } } /** * Handle a dropped image */ - private void on_drag_drop_received(WebKit.JavascriptResult result) { + private void on_drag_drop_received(GLib.Variant? parameters) { + var dict = new GLib.VariantDict(parameters); + string file_name = dict.lookup_value( + "fileName", GLib.VariantType.STRING + ).get_string(); + string file_name_unescaped = GLib.Uri.unescape_string(file_name); - try { - JSC.Value object = result.get_js_value(); - string filename = Util.JS.to_string( - Util.JS.get_property(object, "fileName") - ); - string filename_unescaped = GLib.Uri.unescape_string(filename); + string file_type = dict.lookup_value( + "fileType", GLib.VariantType.STRING + ).get_string(); - string file_type = Util.JS.to_string( - Util.JS.get_property(object, "fileType") - ); + string content_base64 = dict.lookup_value( + "content", GLib.VariantType.STRING + ).get_string(); + uint8[] image = GLib.Base64.decode(content_base64); - string content_base64 = Util.JS.to_string( - Util.JS.get_property(object, "content") - ); - uint8[] image = GLib.Base64.decode(content_base64); + if (image.length == 0) { + warning("%s is empty", file_name); + return; + } - if (image.length == 0) { - warning("%s is empty", filename); - return; - } - - // A simple check to see if the file looks like an image. A problem here - // will be this accepting types which won't be supported by WebKit - // or recipients. - if (file_type.index_of("image/") == 0) { - image_file_dropped(filename_unescaped, file_type, image); - } - } catch (Util.JS.Error err) { - debug("Could not get deceptive link param: %s", err.message); + // A simple check to see if the file looks like an image. A problem here + // will be this accepting types which won't be supported by WebKit + // or recipients. + if (file_type.index_of("image/") == 0) { + image_file_dropped(file_name_unescaped, file_type, image); } } } diff --git a/ui/composer-web-view.js b/ui/composer-web-view.js index ca918990..4fe34ad0 100644 --- a/ui/composer-web-view.js +++ b/ui/composer-web-view.js @@ -1,6 +1,6 @@ /* - * Copyright 2016 Software Freedom Conservancy Inc. - * Copyright 2016 Michael Gratton + * Copyright © 2016 Software Freedom Conservancy Inc. + * Copyright © 2016-2020 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. @@ -35,6 +35,9 @@ ComposerPageState.prototype = { this.nextSelectionId = 0; this.cursorContext = null; + this._cursorContextChanged = MessageSender("cursor_context_changed"); + this._dragDropReceived = MessageSender("drag_drop_received"); + document.addEventListener("click", function(e) { if (e.target.tagName == "A") { e.preventDefault(); @@ -99,7 +102,9 @@ ComposerPageState.prototype = { }, true); // Handle file drag & drop - document.body.addEventListener("drop", state.handleFileDrop, true); + document.body.addEventListener("drop", function(e) { + state.handleFileDrop(e); + }, true); document.body.addEventListener("allowDrop", function(e) { ev.preventDefault(); }, true); @@ -346,9 +351,7 @@ ComposerPageState.prototype = { let newContext = new EditContext(cursor); if (!newContext.equals(this.cursorContext)) { this.cursorContext = newContext; - window.webkit.messageHandlers.cursorContextChanged.postMessage( - newContext.encode() - ); + this._cursorContextChanged(newContext.encode()); } } @@ -396,13 +399,14 @@ ComposerPageState.prototype = { continue; const reader = new FileReader(); + const state = this; reader.onload = (function(filename, imageType) { return function(loadEvent) { // Remove prefixed file type and encoding type var parts = loadEvent.target.result.split(","); if (parts.length < 2) return; - window.webkit.messageHandlers.dragDropReceived.postMessage({ + state._dragDropReceived({ fileName: encodeURIComponent(filename), fileType: imageType, content: parts[1] From 7b0146274ccaad18115a66ded6753cb68bd22357 Mon Sep 17 00:00:00 2001 From: Michael Gratton Date: Fri, 28 Aug 2020 11:45:35 +1000 Subject: [PATCH 10/18] =?UTF-8?q?Conversation.WebView:=20Convert=20to=20us?= =?UTF-8?q?ing=20messages=20for=20JS=20=E2=86=92=20client=20comms?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../conversation-web-view.vala | 69 +++++++++---------- ui/conversation-web-view.js | 4 +- 2 files changed, 37 insertions(+), 36 deletions(-) diff --git a/src/client/conversation-viewer/conversation-web-view.vala b/src/client/conversation-viewer/conversation-web-view.vala index d77af642..a1ba21a6 100644 --- a/src/client/conversation-viewer/conversation-web-view.vala +++ b/src/client/conversation-viewer/conversation-web-view.vala @@ -1,6 +1,6 @@ /* - * Copyright 2016 Software Freedom Conservancy Inc. - * Copyright 2017 Michael Gratton + * Copyright © 2016 Software Freedom Conservancy Inc. + * Copyright © 2017-2020 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. @@ -9,7 +9,7 @@ public class ConversationWebView : Components.WebView { - private const string DECEPTIVE_LINK_CLICKED = "deceptiveLinkClicked"; + private const string DECEPTIVE_LINK_CLICKED = "deceptive_link_clicked"; // Key codes we don't forward on to the super class on key press // since we want to override them elsewhere, especially @@ -221,48 +221,47 @@ public class ConversationWebView : Components.WebView { } private void init() { - register_message_handler( + register_message_callback( DECEPTIVE_LINK_CLICKED, on_deceptive_link_clicked ); this.notify["preferred-height"].connect(() => queue_resize()); } - private void on_deceptive_link_clicked(WebKit.JavascriptResult result) { - try { - JSC.Value object = result.get_js_value(); - uint reason = (uint) Util.JS.to_int32( - Util.JS.get_property(object, "reason") - ); + private void on_deceptive_link_clicked(GLib.Variant? parameters) { + var dict = new GLib.VariantDict(parameters); + uint reason = (uint) dict.lookup_value( + "reason", GLib.VariantType.DOUBLE + ).get_double(); - string href = Util.JS.to_string( - Util.JS.get_property(object, "href") - ); + string href = dict.lookup_value( + "href", GLib.VariantType.STRING + ).get_string(); - string text = Util.JS.to_string( - Util.JS.get_property(object, "text") - ); + string text = dict.lookup_value( + "text", GLib.VariantType.STRING + ).get_string(); - JSC.Value js_location = Util.JS.get_property(object, "location"); + Gdk.Rectangle location = Gdk.Rectangle(); + var location_dict = new GLib.VariantDict( + dict.lookup_value("location", GLib.VariantType.VARDICT) + ); + location.x = (int) location_dict.lookup_value( + "x", GLib.VariantType.DOUBLE + ).get_double(); + location.y = (int) location_dict.lookup_value( + "y", GLib.VariantType.DOUBLE + ).get_double(); + location.width = (int) location_dict.lookup_value( + "width", GLib.VariantType.DOUBLE + ).get_double(); + location.height = (int) location_dict.lookup_value( + "height", GLib.VariantType.DOUBLE + ).get_double(); - Gdk.Rectangle location = Gdk.Rectangle(); - location.x = Util.JS.to_int32( - Util.JS.get_property(js_location, "x") - ); - location.y = Util.JS.to_int32( - Util.JS.get_property(js_location, "y") - ); - location.width = Util.JS.to_int32( - Util.JS.get_property(js_location, "width") - ); - location.height = Util.JS.to_int32( - Util.JS.get_property(js_location, "height") - ); - - deceptive_link_clicked((DeceptiveText) reason, text, href, location); - } catch (Util.JS.Error err) { - debug("Could not get deceptive link param: %s", err.message); - } + deceptive_link_clicked( + (DeceptiveText) reason, text, href, location + ); } } diff --git a/ui/conversation-web-view.js b/ui/conversation-web-view.js index 451db288..1d730d47 100644 --- a/ui/conversation-web-view.js +++ b/ui/conversation-web-view.js @@ -26,6 +26,8 @@ ConversationPageState.prototype = { init: function() { PageState.prototype.init.apply(this, []); + this._deceptiveLinkClicked = MessageSender("deceptive_link_clicked"); + let state = this; document.addEventListener("click", function(e) { if (e.target.tagName == "A" && @@ -267,7 +269,7 @@ ConversationPageState.prototype = { let reason = ConversationPageState.isDeceptiveText(text, href); if (reason != ConversationPageState.NOT_DECEPTIVE) { cancelClick = true; - window.webkit.messageHandlers.deceptiveLinkClicked.postMessage({ + this._deceptiveLinkClicked({ reason: reason, text: text, href: href, From 7950ce50c6bdb6ca77561a00d1afb9bc689278cf Mon Sep 17 00:00:00 2001 From: Michael Gratton Date: Fri, 28 Aug 2020 11:59:11 +1000 Subject: [PATCH 11/18] GearyWebExtension: Untangle extension and JS interaction a bit Move selection changed event listener into JS so it doesn't have to cross the JS/native boundary twice. Move sending remote load blocked from JS to the extension since we can do that directly now and again so the JS/native boundary doesn't need to be double-crossed again. --- .../web-process/web-process-extension.vala | 60 ++----------------- ui/components-web-view.js | 8 +-- 2 files changed, 8 insertions(+), 60 deletions(-) diff --git a/src/client/web-process/web-process-extension.vala b/src/client/web-process/web-process-extension.vala index 31f2b0f0..6eed7746 100644 --- a/src/client/web-process/web-process-extension.vala +++ b/src/client/web-process/web-process-extension.vala @@ -77,7 +77,10 @@ public class GearyWebExtension : Object { if (should_load_remote_images(page)) { should_load = true; } else { - remote_image_load_blocked(page); + page.send_message_to_view.begin( + new WebKit.UserMessage("remote_image_load_blocked", null), + null + ); } } @@ -99,54 +102,6 @@ public class GearyWebExtension : Object { return should_load; } - private void remote_image_load_blocked(WebKit.WebPage page) { - WebKit.Frame frame = page.get_main_frame(); - JSC.Context context = frame.get_js_context(); - try { - execute_script( - context, - "geary.remoteImageLoadBlocked();", - GLib.Log.FILE, - GLib.Log.METHOD, - GLib.Log.LINE - ); - } catch (Error err) { - debug( - "Error calling PageState::remoteImageLoadBlocked: %s", - err.message - ); - } - } - - private void selection_changed(WebKit.WebPage page) { - WebKit.Frame frame = page.get_main_frame(); - JSC.Context context = frame.get_js_context(); - try { - execute_script( - context, - "geary.selectionChanged();", - GLib.Log.FILE, - GLib.Log.METHOD, - GLib.Log.LINE - ); - } catch (Error err) { - debug("Error calling PageStates::selectionChanged: %s", err.message); - } - } - - private JSC.Value execute_script(JSC.Context context, - string script, - string file_name, - string method_name, - int line_number) - throws Util.JS.Error { - JSC.Value ret = context.evaluate_with_source_uri( - script, -1, "geary:%s/%s".printf(file_name, method_name), line_number - ); - Util.JS.check_exception(context); - return ret; - } - private WebKit.UserMessage to_exception_message(string? name, string? message, string? backtrace = null, @@ -208,13 +163,6 @@ public class GearyWebExtension : Object { page.console_message_sent.connect(on_console_message); page.send_request.connect(on_send_request); - // XXX investigate whether the earliest supported - // version of WK supports the DOM "selectionchanged" - // event, and if so use that rather that doing it in - // here in the extension - page.get_editor().selection_changed.connect(() => { - selection_changed(page); - }); page.user_message_received.connect(on_page_message_received); } diff --git a/ui/components-web-view.js b/ui/components-web-view.js index 29b6acd5..d0998a67 100644 --- a/ui/components-web-view.js +++ b/ui/components-web-view.js @@ -22,7 +22,6 @@ PageState.prototype = { this._selectionChanged = MessageSender("selection_changed"); this._contentLoaded = MessageSender("content_loaded"); - this._remoteImageLoadBlocked = MessageSender("remote_image_load_blocked"); this._preferredHeightChanged = MessageSender("preferred_height_changed"); this._commandStackChanged = MessageSender("command_stack_changed"); this._documentModified = MessageSender("document_modified"); @@ -46,6 +45,10 @@ PageState.prototype = { state.loaded(); }); + document.addEventListener("selectionchange", function(e) { + state.selectionChanged(); + }); + // Coalesce multiple calls to updatePreferredHeight using a // timeout to avoid the overhead of multiple JS messages sent // to the app and hence view multiple resizes being queued. @@ -148,9 +151,6 @@ PageState.prototype = { stopBodyObserver: function() { this.bodyObserver.disconnect(); }, - remoteImageLoadBlocked: function() { - this._remoteImageLoadBlocked(); - }, /** * Sends "preferredHeightChanged" message if it has changed. */ From eba82a1fbda18c2e567bd143862e7d687982b973 Mon Sep 17 00:00:00 2001 From: Michael Gratton Date: Fri, 28 Aug 2020 12:01:22 +1000 Subject: [PATCH 12/18] GearyWebExtension: Trivial code clean up --- src/client/web-process/web-process-extension.vala | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/client/web-process/web-process-extension.vala b/src/client/web-process/web-process-extension.vala index 6eed7746..6785903e 100644 --- a/src/client/web-process/web-process-extension.vala +++ b/src/client/web-process/web-process-extension.vala @@ -1,5 +1,5 @@ /* - * Copyright 2016 Michael Gratton + * Copyright © 2016-2020 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. @@ -13,9 +13,9 @@ public void webkit_web_extension_initialize_with_user_data(WebKit.WebExtension e bool logging_enabled = data.get_boolean(); Geary.Logging.init(); - GLib.Log.set_writer_func(Geary.Logging.default_log_writer); if (logging_enabled) { - Geary.Logging.log_to(stdout); + GLib.Log.set_writer_func(Geary.Logging.default_log_writer); + Geary.Logging.log_to(GLib.stdout); } debug("Initialising..."); @@ -50,7 +50,6 @@ public class GearyWebExtension : Object { extension.page_created.connect(on_page_created); } - // XXX Conditionally enable while we still depend on WK2 <2.12 private void on_console_message(WebKit.WebPage page, WebKit.ConsoleMessage message) { string source = message.get_source_id(); From 47b134a04eeb502fadb80e6ae2340178645396bd Mon Sep 17 00:00:00 2001 From: Michael Gratton Date: Fri, 28 Aug 2020 12:05:23 +1000 Subject: [PATCH 13/18] Components.WebView: Remove now-unused message handler infrastructure --- .../components/components-web-view.vala | 30 +------------------ 1 file changed, 1 insertion(+), 29 deletions(-) diff --git a/src/client/components/components-web-view.vala b/src/client/components/components-web-view.vala index a5cdfe33..904c5358 100644 --- a/src/client/components/components-web-view.vala +++ b/src/client/components/components-web-view.vala @@ -195,9 +195,6 @@ public abstract class Components.WebView : WebKit.WebView, Geary.BaseInterface { } - /** Delegate for UserContentManager message callbacks. */ - public delegate void JavaScriptMessageHandler(WebKit.JavascriptResult js_result); - /** * Delegate for message handler callbacks. * @@ -279,8 +276,6 @@ public abstract class Components.WebView : WebKit.WebView, Geary.BaseInterface { private Gee.Map internal_resources = new Gee.HashMap(); - private Gee.List registered_message_handlers = - new Gee.LinkedList(); private Gee.Map message_handlers = new Gee.HashMap(); @@ -357,7 +352,7 @@ public abstract class Components.WebView : WebKit.WebView, Geary.BaseInterface { * The new view will use the same WebProcess, settings and content * manager as the given related view's. * - * @see WebKit.WebView.with_related_view + * @see WebKit.WebView.WebView.with_related_view */ protected WebView.with_related_view(Application.Configuration config, WebView related) { @@ -375,10 +370,6 @@ public abstract class Components.WebView : WebKit.WebView, Geary.BaseInterface { } public override void destroy() { - foreach (ulong id in this.registered_message_handlers) { - this.user_content_manager.disconnect(id); - } - this.registered_message_handlers.clear(); this.message_handlers.clear(); base.destroy(); } @@ -570,25 +561,6 @@ public abstract class Components.WebView : WebKit.WebView, Geary.BaseInterface { return ret_value; } - /** - * Convenience function for registering and connecting JS messages. - */ - protected inline void register_message_handler(string name, - JavaScriptMessageHandler handler) { - // XXX can't use the delegate directly, see b.g.o Bug - // 604781. However the workaround below creates a circular - // reference, causing WebView instances to leak. So to - // work around that we need to record handler ids and - // disconnect them when being destroyed. - ulong id = this.user_content_manager.script_message_received[name].connect( - (result) => { handler(result); } - ); - this.registered_message_handlers.add(id); - if (!this.user_content_manager.register_script_message_handler(name)) { - debug("Failed to register script message handler: %s", name); - } - } - /** * Registers a callback for a specific WebKit user message. */ From 92e842bf083d3185e6ee94b09eab710fb2374fac Mon Sep 17 00:00:00 2001 From: Michael Gratton Date: Fri, 28 Aug 2020 12:06:07 +1000 Subject: [PATCH 14/18] ConversationViewer.ConversationMessage: Fix valadoc warning --- src/client/conversation-viewer/conversation-message.vala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/client/conversation-viewer/conversation-message.vala b/src/client/conversation-viewer/conversation-message.vala index a11ece01..109c4a1c 100644 --- a/src/client/conversation-viewer/conversation-message.vala +++ b/src/client/conversation-viewer/conversation-message.vala @@ -666,7 +666,7 @@ public class ConversationMessage : Gtk.Grid, Geary.BaseInterface { /** * Adds a set of internal resources to web_view. * - * @see add_internal_resource + * @see Components.WebView.add_internal_resources */ public void add_internal_resources(Gee.Map res) { if (this.web_view == null) From 0609fbc3d7cec05a4e5fbf4c5ca9377ea802344b Mon Sep 17 00:00:00 2001 From: Michael Gratton Date: Fri, 28 Aug 2020 12:06:36 +1000 Subject: [PATCH 15/18] Util.JS: Remove now-unused code --- src/client/util/util-js.vala | 50 ------------------------------ test/client/util/util-js-test.vala | 11 ------- 2 files changed, 61 deletions(-) diff --git a/src/client/util/util-js.vala b/src/client/util/util-js.vala index 095f9da4..193b3c7a 100644 --- a/src/client/util/util-js.vala +++ b/src/client/util/util-js.vala @@ -364,56 +364,6 @@ namespace Util.JS { return value; } - /** - * Escapes a string so as to be safe to use as a JS string literal. - * - * This does not append opening or closing quotes. - */ - public string escape_string(string value) { - StringBuilder builder = new StringBuilder.sized(value.length); - for (int i = 0; i < value.length; i++) { - if (value.valid_char(i)) { - unichar c = value.get_char(i); - switch (c) { - case '\x00': - builder.append("\x00"); - break; - case '\'': - builder.append("\\\'"); - break; - case '"': - builder.append("\\\""); - break; - case '\\': - builder.append("\\\\"); - break; - case '\n': - builder.append("\\n"); - break; - case '\r': - builder.append("\\r"); - break; - case '\x0b': - builder.append("\x0b"); - break; - case '\t': - builder.append("\\t"); - break; - case '\b': - builder.append("\\b"); - break; - case '\f': - builder.append("\\f"); - break; - default: - builder.append_unichar(c); - break; - } - } - } - return (string) builder.data; - } - /** * Convenience method for returning a new Callable instance. */ diff --git a/test/client/util/util-js-test.vala b/test/client/util/util-js-test.vala index f1da043d..9fea3cd7 100644 --- a/test/client/util/util-js-test.vala +++ b/test/client/util/util-js-test.vala @@ -13,7 +13,6 @@ public class Util.JS.Test : TestCase { public Test() { base("Util.JS.Test"); - add_test("escape_string", escape_string); add_test("to_variant", to_variant); add_test("to_value", to_value); } @@ -26,16 +25,6 @@ public class Util.JS.Test : TestCase { this.context = null; } - public void escape_string() throws GLib.Error { - assert(Util.JS.escape_string("\n") == """\n"""); - assert(Util.JS.escape_string("\r") == """\r"""); - assert(Util.JS.escape_string("\t") == """\t"""); - assert(Util.JS.escape_string("\'") == """\'"""); - assert(Util.JS.escape_string("\"") == """\""""); - - assert(Util.JS.escape_string("something…\n") == """something…\n"""); - } - public void to_variant() throws GLib.Error { assert_equal( value_to_variant(new JSC.Value.null(this.context)).print(true), From 1d80ed2034512aca7e355921c0b942d4cf651b94 Mon Sep 17 00:00:00 2001 From: Michael Gratton Date: Fri, 28 Aug 2020 12:07:59 +1000 Subject: [PATCH 16/18] ComposerPageState: Use CSS for managing focus with composer body parts Now that the `:focus-within` pseudoclass is supported, use this rather than some custom JS to update custom HTML classes. This also prevents spurious mutation events from firing. --- test/js/composer-page-state-test.vala | 24 ++++++++---------------- ui/composer-web-view.css | 6 +++--- ui/composer-web-view.js | 25 ------------------------- 3 files changed, 11 insertions(+), 44 deletions(-) diff --git a/test/js/composer-page-state-test.vala b/test/js/composer-page-state-test.vala index 1a2e2df3..5a0a8b3c 100644 --- a/test/js/composer-page-state-test.vala +++ b/test/js/composer-page-state-test.vala @@ -11,7 +11,7 @@ class Composer.PageStateTest : Components.WebViewTestCase { """
%s


"""; public const string DIRTY_BODY_TEMPLATE = """ -
%s


+
%s


"""; public const string CLEAN_BODY_TEMPLATE = """
%s


"""; @@ -227,7 +227,7 @@ some text } } - public void clean_content() throws Error { + public void clean_content() throws GLib.Error { // XXX split these up into multiple tests load_body_fixture(""" http://example1.com @@ -257,20 +257,12 @@ unknown://example6.com I can send email through smtp.gmail.com:587 or through https://www.gmail.com/ """; - try { - run_javascript("geary.cleanContent();"); - string result = Util.JS.to_string( - run_javascript("window.document.body.innerHTML;") - .get_js_value() - ); - assert(result == DIRTY_BODY_TEMPLATE.printf(expected)); - } catch (Util.JS.Error err) { - print("Util.JS.Error: %s\n", err.message); - assert_not_reached(); - } catch (Error err) { - print("WKError: %s\n", err.message); - assert_not_reached(); - } + run_javascript("geary.cleanContent();"); + string result = Util.JS.to_string( + run_javascript("window.document.body.innerHTML;") + .get_js_value() + ); + assert_equal(result, DIRTY_BODY_TEMPLATE.printf(expected)); } public void get_html() throws Error { diff --git a/ui/composer-web-view.css b/ui/composer-web-view.css index 3cecfb3b..07ae6869 100644 --- a/ui/composer-web-view.css +++ b/ui/composer-web-view.css @@ -43,12 +43,12 @@ body > div#geary-quote { padding: 6px !important; } -body > div.geary-focus { +body > div:focus-within { background-color: white; } -body > div#geary-signature.geary-focus, -body > div#geary-quote.geary-focus { +body > div#geary-signature:focus-within, +body > div#geary-quote:focus-within { outline: 1px dashed #ccc !important; } diff --git a/ui/composer-web-view.js b/ui/composer-web-view.js index 4fe34ad0..5ee4105e 100644 --- a/ui/composer-web-view.js +++ b/ui/composer-web-view.js @@ -123,7 +123,6 @@ ComposerPageState.prototype = { // Focus within the HTML document document.body.focus(); - this.updateFocusClass(this.bodyPart); // Set text cursor at appropriate position let cursor = document.getElementById("cursormarker"); @@ -354,30 +353,6 @@ ComposerPageState.prototype = { this._cursorContextChanged(newContext.encode()); } } - - while (cursor != null) { - let parent = cursor.parentNode; - if (parent == document.body) { - this.updateFocusClass(cursor); - break; - } - cursor = parent; - } - }, - /** - * Work around WebKit note yet supporting :focus-inside pseudoclass. - */ - updateFocusClass: function(newFocus) { - if (this.focusedPart != null) { - this.focusedPart.classList.remove("geary-focus"); - this.focusedPart = null; - } - if (newFocus == this.bodyPart || - newFocus == this.signaturePart || - newFocus == this.quotePart) { - this.focusedPart = newFocus; - this.focusedPart.classList.add("geary-focus"); - } }, containedInPart: function(target) { let inPart = false; From 3f7c054db085a846b550c2cbc9fb71b352add2b9 Mon Sep 17 00:00:00 2001 From: Michael Gratton Date: Sat, 29 Aug 2020 12:57:08 +1000 Subject: [PATCH 17/18] build: Bump WebKitGTK min version to include UserMessage support --- meson.build | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/meson.build b/meson.build index 240eacaa..9b55b24d 100644 --- a/meson.build +++ b/meson.build @@ -53,7 +53,7 @@ valac = meson.get_compiler('vala') target_vala = '0.48.6' target_glib = '2.64' target_gtk = '3.24.7' -target_webkit = '2.26' +target_webkit = '2.28' if not valac.version().version_compare('>=' + target_vala) error('Vala does not meet minimum required version: ' + target_vala) From 4db6d01e2356a5c9542ee394ecfaea5aaf6ccab8 Mon Sep 17 00:00:00 2001 From: Michael Gratton Date: Tue, 13 Oct 2020 18:49:57 +1100 Subject: [PATCH 18/18] client: Remove perf relnote, it's not really that noteworthy --- desktop/org.gnome.Geary.appdata.xml.in.in | 8 -------- 1 file changed, 8 deletions(-) diff --git a/desktop/org.gnome.Geary.appdata.xml.in.in b/desktop/org.gnome.Geary.appdata.xml.in.in index 471b69c4..7c1ddcab 100644 --- a/desktop/org.gnome.Geary.appdata.xml.in.in +++ b/desktop/org.gnome.Geary.appdata.xml.in.in @@ -88,14 +88,6 @@ geary - - -

Enhancements included in this release:

-
    -
  • Conversation loading performance improvements
    • -
- -

Enhancements included in this release: