[geary/mjog/1170-webview-remote-resource-loading: 3/3] client: Fix remote images not being loaded for remembered senders/email




commit 946f6d786f4f4e21486ff83eb66cd1f4ba8c9a37
Author: Michael Gratton <mike vee net>
Date:   Sun Apr 4 20:51:25 2021 +1000

    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

 src/client/components/components-web-view.vala     | 39 +++++++++-------
 .../conversation-viewer/conversation-message.vala  |  8 ++--
 src/client/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 ff6b379f6..fe3c036d2 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 d0d001ad6..c15f3f8fc 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 33a4fc26c..c36c4e28d 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<string>(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<string>(
+                    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 6fa5e0b1b..8221c2319 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;
             }
         }
     },


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]