[geary/wip/728002-webkit2: 76/105] Re-implement remote image loading management for WebKit2.



commit 8e71fd9fa7d905e97c5b17a479d374ddb3e08d05
Author: Michael James Gratton <mike vee net>
Date:   Fri Nov 25 16:30:50 2016 +1100

    Re-implement remote image loading management for WebKit2.
    
    * src/client/components/client-web-view.vala (ClientWebView): Register
      for new remoteImageLoadBlocked message from JS. Add new
      ::remote_image_load_blocked signal to notify when a remote image load
      was blocked. Add ::allow_remote_image_loading and ::load_remote_images
      methods to allow th app to manage image loading state.
    
    * src/client/conversation-viewer/conversation-email.vala
      (ConversationEmail): Determine up front whether the message view should
      load remote images and flag that, rather than passing through the email
      flag and the contact store.
    
    * src/client/conversation-viewer/conversation-message.vala
      (ConversationMessage): Rmeove contact_store and
      always_load_remote_images properties and related code, just let the
      parent email view advise whether remote images should be initially
      loaded and act accordingly. Look up to ClientWebView signal and methods
      to handle manual remote image loading management by the user. Remove
      some obsolete code.
    
    * src/client/web-process/web-process-extension.vala (GearyWebExtension):
      Replace allow_prefix implementation for remote image management with
      explicit signalling to/from the application via JavaScript.
    
    * ui/client-web-view.js: Add a new PageState object, add props and method
      to implement remote image loading management.
    
    * ui/client-web-view-allow-remote-images.js: Monkeypatch to be used when
      remote images should be loaded by default.
    
    * ui/CMakeLists.txt: Include new JS file.
    
    * src/client/conversation-viewer/conversation-web-view.vala,
      src/client/web-process/util-conversation.vala,
      src/client/web-process/util-webkit.vala: Remove obsolete code.

 src/client/components/client-web-view.vala         |   41 +++++++++-
 .../conversation-viewer/conversation-email.vala    |   14 ++-
 .../conversation-viewer/conversation-message.vala  |   49 +++--------
 .../conversation-viewer/conversation-web-view.vala |   10 +--
 src/client/web-process/util-conversation.vala      |   90 +-------------------
 src/client/web-process/util-webkit.vala            |   16 ----
 src/client/web-process/web-process-extension.vala  |   70 +++++++++++-----
 ui/CMakeLists.txt                                  |    1 +
 ui/client-web-view-allow-remote-images.js          |   11 +++
 ui/client-web-view.js                              |   19 ++++
 10 files changed, 144 insertions(+), 177 deletions(-)
---
diff --git a/src/client/components/client-web-view.vala b/src/client/components/client-web-view.vala
index 98cb5fb..3830319 100644
--- a/src/client/components/client-web-view.vala
+++ b/src/client/components/client-web-view.vala
@@ -15,14 +15,21 @@ public class ClientWebView : WebKit.WebView {
     public const string CID_PREFIX = "cid:";
 
     private const string PREFERRED_HEIGHT_MESSAGE = "preferredHeightChanged";
+    private const string REMOTE_IMAGE_LOAD_BLOCKED_MESSAGE = "remoteImageLoadBlocked";
     private const double ZOOM_DEFAULT = 1.0;
     private const double ZOOM_FACTOR = 0.1;
 
     private static WebKit.UserScript? script = null;
+    private static WebKit.UserScript? allow_remote_images = null;
 
     public static void load_scripts(GearyApplication app)
         throws Error {
-        ClientWebView.script = load_app_script(app, "client-web-view.js");
+        ClientWebView.script = load_app_script(
+            app, "client-web-view.js"
+        );
+        ClientWebView.allow_remote_images = load_app_script(
+            app, "client-web-view-allow-remote-images.js"
+        );
     }
 
     /** Loads an application-specific WebKit stylesheet. */
@@ -83,6 +90,7 @@ public class ClientWebView : WebKit.WebView {
         }
         JS.Value? err = null;
         return (int) context.to_number(value, out err);
+        // XXX unref result
     }
 
     private static inline uint to_wk2_font_size(Pango.FontDescription font) {
@@ -142,6 +150,9 @@ public class ClientWebView : WebKit.WebView {
     /** Emitted when the web view has loaded an inline part. */
     public signal void inline_resource_loaded(string cid);
 
+    /** Emitted when a remote image load was disallowed. */
+    public signal void remote_image_load_blocked();
+
 
     public ClientWebView(WebKit.UserContentManager? custom_manager = null) {
         WebKit.Settings setts = new WebKit.Settings();
@@ -183,8 +194,13 @@ public class ClientWebView : WebKit.WebView {
                     debug("Could not get preferred height: %s", err.message);
                 }
             });
+        content_manager.script_message_received[REMOTE_IMAGE_LOAD_BLOCKED_MESSAGE].connect(
+            (result) => {
+                remote_image_load_blocked();
+            });
 
         register_message_handler(PREFERRED_HEIGHT_MESSAGE);
+        register_message_handler(REMOTE_IMAGE_LOAD_BLOCKED_MESSAGE);
 
         GearyApplication.instance.config.bind(Configuration.CONVERSATION_VIEWER_ZOOM_KEY, this, 
"zoom_level");
         this.scroll_event.connect(on_scroll_event);
@@ -209,6 +225,29 @@ public class ClientWebView : WebKit.WebView {
     }
 
     /**
+     * Allows loading any remote images found during page load.
+     *
+     * This must be called before HTML content is loaded to have any
+     * 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);
+    }
+
+    /**
+     * Load any remote images previously that were blocked.
+     */
+    public void load_remote_images() {
+        run_javascript.begin("geary.loadRemoteImages();", null);
+    }
+
+    /**
      * Selects all content in the web view.
      */
     public void select_all() {
diff --git a/src/client/conversation-viewer/conversation-email.vala 
b/src/client/conversation-viewer/conversation-email.vala
index cdb0183..99921cc 100644
--- a/src/client/conversation-viewer/conversation-email.vala
+++ b/src/client/conversation-viewer/conversation-email.vala
@@ -443,11 +443,15 @@ public class ConversationEmail : Gtk.Box {
             return;
         }
 
-        this.primary_message = new ConversationMessage(
-            message,
-            contact_store,
-            email.load_remote_images().is_certain()
+        bool load_images = email.load_remote_images().is_certain();
+        Geary.Contact contact = this.contact_store.get_by_rfc822(
+            message.get_primary_originator()
         );
+        if (contact != null)  {
+            load_images |= contact.always_load_remote_images();
+        }
+
+        this.primary_message = new ConversationMessage(message, load_images);
         this.primary_message.web_view.add_inline_resources(cid_resources);
         connect_message_view_signals(this.primary_message);
 
@@ -489,7 +493,7 @@ public class ConversationEmail : Gtk.Box {
         }
         foreach (Geary.RFC822.Message sub_message in sub_messages) {
             ConversationMessage attached_message =
-                new ConversationMessage(sub_message, contact_store, false);
+                new ConversationMessage(sub_message, false);
             connect_message_view_signals(attached_message);
             attached_message.web_view.add_inline_resources(cid_resources);
             this.sub_messages.add(attached_message);
diff --git a/src/client/conversation-viewer/conversation-message.vala 
b/src/client/conversation-viewer/conversation-message.vala
index 97a4448..e945efd 100644
--- a/src/client/conversation-viewer/conversation-message.vala
+++ b/src/client/conversation-viewer/conversation-message.vala
@@ -227,16 +227,10 @@ public class ConversationMessage : Gtk.Grid {
     private MenuModel context_menu_contact;
     private MenuModel? context_menu_inspector = null;
 
-    // The contacts for the message's account
-    private Geary.ContactStore contact_store;
-
     // Address fields that can be search through
     private Gee.List<AddressFlowBoxChild> searchable_addresses =
         new Gee.LinkedList<AddressFlowBoxChild>();
 
-    // Should any remote messages be always loaded and displayed?
-    private bool always_load_remote_images;
-
     // Resource that have been loaded by the web view
     private Gee.Map<string,WebKit.WebResource> resources =
         new Gee.HashMap<string,WebKit.WebResource>();
@@ -247,12 +241,12 @@ public class ConversationMessage : Gtk.Grid {
     private int next_replaced_buffer_number = 0;
 
 
-    /** Fired when the user requests remote images be loaded. */
-    public signal void flag_remote_images();
-
     /** Fired when the user clicks a link in the email. */
     public signal void link_activated(string link);
 
+    /** Fired when the user requests remote images be loaded. */
+    public signal void flag_remote_images();
+
     /** Fired when the user requests remote images be always loaded. */
     public signal void remember_remote_images();
 
@@ -271,11 +265,8 @@ public class ConversationMessage : Gtk.Grid {
      * loading processes.
      */
     public ConversationMessage(Geary.RFC822.Message message,
-                               Geary.ContactStore contact_store,
-                               bool always_load_remote_images) {
+                               bool load_remote_images) {
         this.message = message;
-        this.contact_store = contact_store;
-        this.always_load_remote_images = always_load_remote_images;
 
 #if !GTK_3_20
         // GTK < 3.20+ style workarounds. Keep this in sync with
@@ -375,6 +366,9 @@ public class ConversationMessage : Gtk.Grid {
         // Web view
 
         this.web_view = new ConversationWebView();
+        if (load_remote_images) {
+            this.web_view.allow_remote_image_loading();
+        }
         this.web_view.context_menu.connect(on_context_menu);
         this.web_view.link_activated.connect((link) => {
                 link_activated(link);
@@ -383,6 +377,9 @@ public class ConversationMessage : Gtk.Grid {
         this.web_view.resource_load_started.connect((view, res, req) => {
                 this.resources[res.get_uri()] = res;
             });
+        this.web_view.remote_image_load_blocked.connect(() => {
+                this.remote_images_infobar.show();
+            });
         //this.web_view.selection_changed.connect(on_selection_changed);
         this.web_view.show();
 
@@ -472,29 +469,7 @@ public class ConversationMessage : Gtk.Grid {
                 }
             });
 
-        bool load_images = this.web_view.clean_and_load(body_text ?? "");
-        if (load_images) {
-            bool contact_load = false;
-            Geary.Contact contact = this.contact_store.get_by_rfc822(
-                message.get_primary_originator()
-            );
-            if (contact != null)
-                contact_load = contact.always_load_remote_images();
-            if (!contact_load && !this.always_load_remote_images) {
-                remote_images_infobar.show();
-                load_images = false;
-            }
-
-            // XXX racy
-            this.web_view.notify["load-status"].connect((source, param) => {
-                    if (this.web_view.is_loaded) {
-                        if (load_images) {
-                            show_images(false);
-                        }
-                    }
-                });
-        }
-
+        this.web_view.clean_and_load(body_text ?? "");
     }
 
     /**
@@ -750,7 +725,7 @@ public class ConversationMessage : Gtk.Grid {
     }
 
     private void show_images(bool remember) {
-        this.web_view.show_images();
+        this.web_view.load_remote_images();
         if (remember) {
             flag_remote_images();
         }
diff --git a/src/client/conversation-viewer/conversation-web-view.vala 
b/src/client/conversation-viewer/conversation-web-view.vala
index 9994867..f8ebf6f 100644
--- a/src/client/conversation-viewer/conversation-web-view.vala
+++ b/src/client/conversation-viewer/conversation-web-view.vala
@@ -34,10 +34,9 @@ public class ConversationWebView : ClientWebView {
         base(manager);
     }
 
-    public bool clean_and_load(string html) {
+    public void clean_and_load(string html) {
         // XXX clean me
         load_html(html, null);
-        return false; // XXX Work this thes hit out
     }
 
     public bool has_selection() {
@@ -66,11 +65,4 @@ public class ConversationWebView : ClientWebView {
         // XXX
     }
 
-    /**
-     * XXX
-     */
-    public void show_images() {
-        // XXX
-    }
-
 }
diff --git a/src/client/web-process/util-conversation.vala b/src/client/web-process/util-conversation.vala
index 275cd7d..474d515 100644
--- a/src/client/web-process/util-conversation.vala
+++ b/src/client/web-process/util-conversation.vala
@@ -43,8 +43,7 @@ namespace Util.Conversation {
         return offset_height;
     }
 
-    public string clean_html_markup(WebKit.WebPage page, string text, Geary.RFC822.Message message, out bool 
remote_images) {
-        remote_images = false;
+    public string clean_html_markup(WebKit.WebPage page, string text, Geary.RFC822.Message message) {
         try {
             WebKit.DOM.HTMLElement html = (WebKit.DOM.HTMLElement)
                 page.get_dom_document().document_element;
@@ -114,73 +113,6 @@ namespace Util.Conversation {
             // Now look for the signature.
             wrap_html_signature(page, ref html);
 
-            // Then look for all <img> tags. Inline images are replaced with
-            // data URLs.
-            WebKit.DOM.NodeList inline_list = html.query_selector_all("img");
-            Gee.HashSet<string> inlined_content_ids = new Gee.HashSet<string>();
-            for (ulong i = 0; i < inline_list.length; ++i) {
-                // Get the MIME content for the image.
-                WebKit.DOM.HTMLImageElement img = (WebKit.DOM.HTMLImageElement) inline_list.item(i);
-                string? src = img.get_attribute("src");
-                if (Geary.String.is_empty(src))
-                    continue;
-
-                // if no Content-ID, then leave as-is, but note if a non-data: URI is being used for
-                // purposes of detecting remote images
-                string? content_id = src.has_prefix("cid:") ? src.substring(4) : null;
-                if (Geary.String.is_empty(content_id)) {
-                    remote_images = remote_images || !src.has_prefix("data:");
-
-                    continue;
-                }
-
-                // if image has a Content-ID and it's already been replaced by the image replacer,
-                // drop this tag, otherwise fix up this one with the Base-64 data URI of the image
-                // if (!replaced_content_ids.contains(content_id)) {
-                //     string? filename = message.get_content_filename_by_mime_id(content_id);
-                //     Geary.Memory.Buffer image_content = message.get_content_by_mime_id(content_id);
-                //     Geary.Memory.UnownedBytesBuffer? unowned_buffer =
-                //         image_content as Geary.Memory.UnownedBytesBuffer;
-
-                //     // Get the content type.
-                //     string guess;
-                //     if (unowned_buffer != null)
-                //         guess = ContentType.guess(null, unowned_buffer.to_unowned_uint8_array(), null);
-                //     else
-                //         guess = ContentType.guess(null, image_content.get_uint8_array(), null);
-
-                //     string mimetype = ContentType.get_mime_type(guess);
-
-                //     // Replace the SRC to a data URI, the class to a known label for the popup menu,
-                //     // and the ALT to its filename, if supplied
-                //     img.remove_attribute("src");  // Work around a WebKitGTK+ crash. Bug 764152
-                //     img.set_attribute("src", Util.DOM.assemble_data_uri(mimetype, image_content));
-                //     //img.class_list.add(DATA_IMAGE_CLASS);
-                //     if (!Geary.String.is_empty(filename))
-                //         img.set_attribute("alt", filename);
-
-                //     // stash here so inlined image isn't listed as attachment (esp. if it has no
-                //     // Content-Disposition)
-                //     inlined_content_ids.add(content_id);
-                //     attachment_displayed_inline(content_id);
-                // } else {
-                //     // replaced by data: URI, remove this tag and let the inserted one shine through
-                //     img.parent_element.remove_child(img);
-                // }
-            }
-
-            // Remove any inline images that were referenced through Content-ID
-            foreach (string cid in inlined_content_ids) {
-                try {
-                    string escaped_cid = Geary.HTML.escape_markup(cid);
-                    WebKit.DOM.Element? img = html.query_selector(@"[cid='$escaped_cid']");
-                    if (img != null)
-                        img.parent_element.remove_child(img);
-                } catch (Error error) {
-                    debug("Error removing inlined image: %s", error.message);
-                }
-            }
-
             // Now return the whole message.
             return html.get_outer_html();
         } catch (Error e) {
@@ -213,26 +145,6 @@ namespace Util.Conversation {
         }
     }
 
-    public void show_images(WebKit.WebPage page)
-    throws Error {
-        WebKit.DOM.NodeList nodes =
-            page.get_dom_document().body.get_elements_by_tag_name("img");
-        for (ulong i = 0; i < nodes.length; i++) {
-            WebKit.DOM.Element? element = nodes.item(i) as WebKit.DOM.Element;
-            if (element == null || !element.has_attribute("src"))
-                continue;
-
-            // string src = element.get_attribute("src");
-            // Don't prefix empty src strings since it will cause
-            // e.g. 0px images (commonly found in commercial mailouts)
-            // to be rendered as broken images instead of empty
-            // elements.
-            // if (src.length > 0 && !page.is_always_loaded(src)) {
-            //     element.set_attribute("src", page.allow_prefix + src);
-            // }
-        }
-    }
-
     public string? get_selection_for_quoting(WebKit.WebPage page) {
         string? quote = null;
         // WebKit.DOM.Document document = page.get_dom_document();
diff --git a/src/client/web-process/util-webkit.vala b/src/client/web-process/util-webkit.vala
index b8bb3a1..06ce9af 100644
--- a/src/client/web-process/util-webkit.vala
+++ b/src/client/web-process/util-webkit.vala
@@ -420,20 +420,4 @@ namespace Util.DOM {
         }
     }
 
-    // Returns a URI suitable for an IMG SRC attribute (or elsewhere, potentially) that is the
-    // memory buffer unpacked into a Base-64 encoded data: URI
-    public string assemble_data_uri(string mimetype, Geary.Memory.Buffer buffer) {
-        // attempt to use UnownedBytesBuffer to avoid memcpying a potentially huge buffer only to
-        // free it when the encoding operation is completed
-        string base64;
-        Geary.Memory.UnownedBytesBuffer? unowned_bytes = buffer as Geary.Memory.UnownedBytesBuffer;
-        if (unowned_bytes != null)
-            base64 = Base64.encode(unowned_bytes.to_unowned_uint8_array());
-        else
-            base64 = Base64.encode(buffer.get_uint8_array());
-
-        return "data:%s;base64,%s".printf(mimetype, base64);
-    }
-
 }
-
diff --git a/src/client/web-process/web-process-extension.vala 
b/src/client/web-process/web-process-extension.vala
index 227932e..c3daf52 100644
--- a/src/client/web-process/web-process-extension.vala
+++ b/src/client/web-process/web-process-extension.vala
@@ -29,47 +29,77 @@ public void webkit_web_extension_initialize_with_user_data(WebKit.WebExtension e
 public class GearyWebExtension : Object {
 
 
-    private WebKit.WebExtension extension;
+    private const string CID_PREFIX = "cid:";
+    private const string DATA_PREFIX = "data:";
 
-    private string allow_prefix;
+    private WebKit.WebExtension extension;
 
 
     public GearyWebExtension(WebKit.WebExtension extension) {
         this.extension = extension;
-        this.allow_prefix = random_string(10) + ":";
-
         extension.page_created.connect((extension, web_page) => {
+                // XXX Re-enable when we can depend on WK2 2.12
+                // web_page.console_message_sent.connect(on_console_message);
                 web_page.send_request.connect(on_send_request);
             });
     }
 
+    // XXX Re-enable when we can depend on WK2 2.12
+    // private void on_console_message(WebKit.WebPage page,
+    //                                 WebKit.ConsoleMessage message) {
+    //     debug("[%s] %s %s:%u: %s",
+    //           message.get_level().to_string(),
+    //           message.get_source().to_string(),
+    //           message.get_source_id(),
+    //           message.get_line(),
+    //           message.get_text()
+    //     );
+    // }
+
     private bool on_send_request(WebKit.WebPage page,
                                  WebKit.URIRequest request,
                                  WebKit.URIResponse? response) {
-        const string CID_PREFIX = "cid:";
-        const string DATA_PREFIX = "data:";
-
         bool should_load = false;
         string req_uri = request.get_uri();
-        if (req_uri.has_prefix(CID_PREFIX) |
+        if (req_uri.has_prefix(CID_PREFIX) ||
             req_uri.has_prefix(DATA_PREFIX)) {
+            // Always load images with these prefixes
             should_load = true;
-        } else if (req_uri.has_prefix(this.allow_prefix)) {
-            should_load = true;
-            request.set_uri(req_uri.substring(this.allow_prefix.length));
+        } else {
+            // Only load anything else if remote image loading is
+            // permitted
+            if (should_load_remote_images(page)) {
+                should_load = true;
+            } else {
+                remote_image_load_blocked(page);
+            }
         }
 
         return should_load ? Gdk.EVENT_PROPAGATE : Gdk.EVENT_STOP; // LOL
     }
 
-}
+    private bool should_load_remote_images(WebKit.WebPage page) {
+        WebKit.Frame frame = page.get_main_frame();
+        JS.GlobalContext context = frame.get_javascript_global_context();
+        JS.Value ret = execute_script(context, "geary.allowRemoteImages");
+        // XXX check err here, log it
+        //JS.Value? err;
+        //return context.to_boolean(ret, err);
+        return context.to_boolean(ret);
+    }
+
+    private void remote_image_load_blocked(WebKit.WebPage page) {
+        WebKit.Frame frame = page.get_main_frame();
+        JS.Context context = frame.get_javascript_global_context();
+        execute_script(context, "geary.remoteImageLoadBlocked();");
+    }
+
+    private JS.Value execute_script(JS.Context context, string script) {
+        JS.String js_script = new JS.String.create_with_utf8_cstring(script);
+        // XXX check err here, log it
+        //JS.Value? err;
+        //context.evaluate_script(js_script, null, null, 0, out err);
+        return context.evaluate_script(js_script, null, null, 0, null);
+    }
 
-private string random_string(int length) {
-    // No upper case letters, since request gets lower-cased.
-    string chars = "abcdefghijklmnopqrstuvwxyz";
-    char[] random = new char[length+1]; //leave room for terminating null
-    for (int i = 0; i < length; i++)
-        random[i] = chars[Random.int_range(0, chars.length)];
-    random[length] = '\0'; //make sure the string is null-terminated
-    return (string) random;
 }
diff --git a/ui/CMakeLists.txt b/ui/CMakeLists.txt
index c231807..a47e1b6 100644
--- a/ui/CMakeLists.txt
+++ b/ui/CMakeLists.txt
@@ -6,6 +6,7 @@ set(RESOURCE_LIST
   STRIPBLANKS "account_spinner.glade"
   STRIPBLANKS "certificate_warning_dialog.glade"
               "client-web-view.js"
+              "client-web-view-allow-remote-images.js"
   STRIPBLANKS "composer-headerbar.ui"
   STRIPBLANKS "composer-menus.ui"
   STRIPBLANKS "composer-widget.ui"
diff --git a/ui/client-web-view-allow-remote-images.js b/ui/client-web-view-allow-remote-images.js
new file mode 100644
index 0000000..1fb0560
--- /dev/null
+++ b/ui/client-web-view-allow-remote-images.js
@@ -0,0 +1,11 @@
+/*
+ * Copyright 2016 Michael Gratton <mike vee net>
+ *
+ * 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/client-web-view.js
index b80554e..0b52438 100644
--- a/ui/client-web-view.js
+++ b/ui/client-web-view.js
@@ -9,12 +9,31 @@
  * Application logic for ClientWebView and subclasses.
  */
 
+var PageState = function() { };
+PageState.prototype = {
+    allowRemoteImages: false,
+    loadRemoteImages: function() {
+        this.allowRemoteImages = true;
+        var images = document.getElementsByTagName("IMG");
+        for (var i = 0; i < images.length; i++) {
+            var img = images.item(i);
+            var src = img.src;
+            img.src = "";
+            img.src = src;
+        }
+    },
+    remoteImageLoadBlocked: function() {
+        window.webkit.messageHandlers.remoteImageLoadBlocked.postMessage(null);
+    }
+};
+
 function emitPreferredHeightChanged() {
     window.webkit.messageHandlers.preferredHeightChanged.postMessage(
         window.document.documentElement.offsetHeight
     );
 }
 
+var geary = new PageState();
 window.onload = function() {
     emitPreferredHeightChanged();
 };


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