From 946f6d786f4f4e21486ff83eb66cd1f4ba8c9a37 Mon Sep 17 00:00:00 2001 From: Michael Gratton Date: Sun, 4 Apr 2021 20:51:25 +1000 Subject: [PATCH] client: Fix remote images not being loaded for remembered senders/email Since commit 6a0ad721 landed the client can no longer reply on pre-filling JS page state before loading the message body HTML, since it would be cleared when the JS page state is initialised in the window-object-cleared handler. Instead, set the load remote flag as a GObject data property on the WebKit.WebPage object in the web process via the extension, and consult that when determining whether to allow loading remote content. Fixes #1170 --- .../components/components-web-view.vala | 39 ++++++++------ .../conversation-message.vala | 8 +-- .../web-process/web-process-extension.vala | 53 +++++++++---------- ui/components-web-view.js | 45 ++++++++-------- 4 files changed, 73 insertions(+), 72 deletions(-) diff --git a/src/client/components/components-web-view.vala b/src/client/components/components-web-view.vala index ff6b379f..fe3c036d 100644 --- a/src/client/components/components-web-view.vala +++ b/src/client/components/components-web-view.vala @@ -27,8 +27,9 @@ public abstract class Components.WebView : WebKit.WebView, Geary.BaseInterface { 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__"; + private const string MESSAGE_ENABLE_REMOTE_LOAD = "__enable_remote_load__"; + private const string MESSAGE_EXCEPTION = "__exception__"; + private const string MESSAGE_RETURN_VALUE = "__return__"; // WebKit message handler names private const string COMMAND_STACK_CHANGED = "command_stack_changed"; @@ -252,14 +253,15 @@ public abstract class Components.WebView : WebKit.WebView, Geary.BaseInterface { } /** - * Determines if any remote resources are loaded during page load. + * Specifies whether loading remote resources is currently permitted. * - * This must be set before HTML loaded to have any effect, that - * is, before calling {@link load_html}. Afterwards, you must call - * {@link load_remote_resources} instead. + * If false, any remote resources contained in HTML loaded into + * the view will be blocked. + * + * @see load_remote_resources */ - public bool enable_loading_remote_resources { - get; set; default = false; + public bool is_load_remote_resources_enabled { + get; private set; default = false; } public string document_font { @@ -433,14 +435,19 @@ public abstract class Components.WebView : WebKit.WebView, Geary.BaseInterface { /** * Load any remote resources that were previously blocked. * - * This method will ensure any remote resources that were blocked + * Calling this before calling {@link load_html} will enable any + * remote resources to be loaded as the HTML is loaded. Calling it + * afterwards wil ensure any remote resources that were blocked * during initial HTML page load are now loaded. * - * @see enable_loading_remote_resources + * @see is_load_remote_resources_enabled */ - public void load_remote_resources() { - this.enable_loading_remote_resources = true; - this.call_void.begin(Util.JS.callable("loadRemoteResources"), null); + public async void load_remote_resources(GLib.Cancellable? cancellable) + throws GLib.Error { + this.is_load_remote_resources_enabled = true; + yield this.call_void( + Util.JS.callable(MESSAGE_ENABLE_REMOTE_LOAD), null + ); } /** @@ -639,7 +646,7 @@ public abstract class Components.WebView : WebKit.WebView, Geary.BaseInterface { ); if (response != null) { var response_name = response.name; - if (response_name == MESSAGE_EXCEPTION_NAME) { + if (response_name == MESSAGE_EXCEPTION) { 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; @@ -662,7 +669,7 @@ public abstract class Components.WebView : WebKit.WebView, Geary.BaseInterface { } throw new Util.JS.Error.EXCEPTION(log_message); - } else if (response_name != MESSAGE_RETURN_VALUE_NAME) { + } else if (response_name != MESSAGE_RETURN_VALUE) { throw new Util.JS.Error.TYPE( "Method call %s returned unknown name: %s", target.to_string(), @@ -822,7 +829,7 @@ public abstract class Components.WebView : WebKit.WebView, Geary.BaseInterface { } private bool on_message_received(WebKit.UserMessage message) { - if (message.name == MESSAGE_EXCEPTION_NAME) { + if (message.name == MESSAGE_EXCEPTION) { 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; diff --git a/src/client/conversation-viewer/conversation-message.vala b/src/client/conversation-viewer/conversation-message.vala index d0d001ad..c15f3f8f 100644 --- a/src/client/conversation-viewer/conversation-message.vala +++ b/src/client/conversation-viewer/conversation-message.vala @@ -876,9 +876,9 @@ public class ConversationMessage : Gtk.Grid, Geary.BaseInterface { this.primary_contact != null && this.primary_contact.load_remote_resources ); - this.web_view.enable_loading_remote_resources = ( - this.load_remote_resources || contact_load_images - ); + if (this.load_remote_resources || contact_load_images) { + yield this.web_view.load_remote_resources(load_cancelled); + } show_placeholder_pane(null); @@ -1152,7 +1152,7 @@ public class ConversationMessage : Gtk.Grid, Geary.BaseInterface { this.remote_resources_requested = 0; this.remote_resources_loaded = 0; if (this.web_view != null) { - this.web_view.load_remote_resources(); + this.web_view.load_remote_resources.begin(null); } if (update_email_flag) { flag_remote_images(); diff --git a/src/client/web-process/web-process-extension.vala b/src/client/web-process/web-process-extension.vala index 33a4fc26..c36c4e28 100644 --- a/src/client/web-process/web-process-extension.vala +++ b/src/client/web-process/web-process-extension.vala @@ -33,14 +33,15 @@ 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__"; + private const string MESSAGE_EXCEPTION = "__exception__"; + private const string MESSAGE_ENABLE_REMOTE_LOAD = "__enable_remote_load__"; + private const string MESSAGE_RETURN_VALUE = "__return__"; 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 const string EXTENSION_CLASS_ALLOW_REMOTE_LOAD = "allowRemoteResourceLoad"; private WebKit.WebExtension extension; @@ -88,18 +89,7 @@ public class GearyWebExtension : Object { } private bool should_load_remote_resources(WebKit.WebPage page) { - bool should_load = false; - WebKit.Frame frame = page.get_main_frame(); - JSC.Context context = frame.get_js_context(); - try { - should_load = Util.JS.to_bool(context.get_value(REMOTE_LOAD_VAR)); - } catch (GLib.Error err) { - debug( - "Error checking PageState::allowRemoteImages: %s", - err.message - ); - } - return should_load; + return page.get_data(EXTENSION_CLASS_ALLOW_REMOTE_LOAD) != null; } private WebKit.UserMessage to_exception_message(string? name, @@ -128,7 +118,7 @@ public class GearyWebExtension : Object { detail.insert_value("column_number", new GLib.Variant.uint32(column_number)); } return new WebKit.UserMessage( - MESSAGE_EXCEPTION_NAME, + MESSAGE_EXCEPTION, detail.end() ); } @@ -144,8 +134,6 @@ public class GearyWebExtension : Object { 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; @@ -166,9 +154,23 @@ public class GearyWebExtension : Object { } } - JSC.Value ret = page_state.object_invoke_methodv( - message.name, call_param - ); + JSC.Value page_state = context.get_value(PAGE_STATE_OBJECT_NAME); + JSC.Value? ret = null; + if (message.name == MESSAGE_ENABLE_REMOTE_LOAD) { + page.set_data( + EXTENSION_CLASS_ALLOW_REMOTE_LOAD, + EXTENSION_CLASS_ALLOW_REMOTE_LOAD + ); + if (!page_state.is_undefined()) { + ret = page_state.object_invoke_methodv( + "loadRemoteResources", null + ); + } + } else { + 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 @@ -190,8 +192,8 @@ public class GearyWebExtension : Object { } else { message.send_reply( new WebKit.UserMessage( - MESSAGE_RETURN_VALUE_NAME, - Util.JS.value_to_variant(ret) + MESSAGE_RETURN_VALUE, + ret != null ? Util.JS.value_to_variant(ret) : null ) ); } @@ -267,11 +269,6 @@ public class GearyWebExtension : Object { EXTENSION_CLASS_VAR, new JSC.Value.object(context, extension_class, extension_class) ); - - context.set_value( - REMOTE_LOAD_VAR, - new JSC.Value.boolean(context, false) - ); } } diff --git a/ui/components-web-view.js b/ui/components-web-view.js index 6fa5e0b1..8221c231 100644 --- a/ui/components-web-view.js +++ b/ui/components-web-view.js @@ -73,31 +73,28 @@ PageState.prototype = { this._contentLoaded(); }, loadRemoteResources: function() { - if (window._gearyAllowRemoteResourceLoads == false) { - window._gearyAllowRemoteResourceLoads = true; - const TYPES = "*[src], *[srcset]"; - for (const element of document.body.querySelectorAll(TYPES)) { - let src = ""; - try { - src = element.src; - } catch (e) { - // fine - } - if (src != "") { - element.src = ""; - element.src = src; - } + const TYPES = "*[src], *[srcset]"; + for (const element of document.body.querySelectorAll(TYPES)) { + let src = ""; + try { + src = element.src; + } catch (e) { + // fine + } + if (src != "") { + element.src = ""; + element.src = src; + } - let srcset = ""; - try { - srcset = element.srcset; - } catch (e) { - // fine - } - if (srcset != "") { - element.srcset = ""; - element.srcset = srcset; - } + let srcset = ""; + try { + srcset = element.srcset; + } catch (e) { + // fine + } + if (srcset != "") { + element.srcset = ""; + element.srcset = srcset; } } },