[geary/wip/765516-gtk-widget-conversation-viewer: 109/112] Fix most conversation web view height issues.

[Michael Gratton <mjog src gnome org>]

commit 9ad7aad8e1b08dc1584e402681bfcadeabbcc8cd
Author: Michael James Gratton <mike vee net>
Date:   Fri Aug 12 00:07:55 2016 +1000

    Fix most conversation web view height issues.
    
    * src/client/conversation-viewer/conversation-message.vala
      (ConversationMessage): Remove ::is_loading_complete prop since it was
      neveer accurate, replace uses with ConversationWebView::is_height_valid
      instead. Hook up unset_controllable_quotes to web_view.size_allocate so
      quotes and their containers have accurate heights.
    
    * src/client/conversation-viewer/conversation-web-view.vala
      (ConversationWebView): Make ::is_height_valid a property in case we
      want to get notify events about it in the future. Don't use an arbirary
      max hieght to determine if the HTML's height is valid, use what it
      thinks its width is instead.

 .../conversation-viewer/conversation-listbox.vala  |    4 +-
 .../conversation-viewer/conversation-message.vala  |   32 +++++++++++------
 .../conversation-viewer/conversation-web-view.vala |   36 +++++++++-----------
 3 files changed, 38 insertions(+), 34 deletions(-)
---
diff --git a/src/client/conversation-viewer/conversation-listbox.vala 
b/src/client/conversation-viewer/conversation-listbox.vala
index 2a1e2bb..a334cf2 100644
--- a/src/client/conversation-viewer/conversation-listbox.vala
+++ b/src/client/conversation-viewer/conversation-listbox.vala
@@ -98,7 +98,7 @@ public class ConversationListBox : Gtk.ListBox {
             // message has a non-trivial height, and then wait for it
             // to be reallocated, so that it picks up the web_view's
             // height.
-            if (view.primary_message.is_loading_complete) {
+            if (view.primary_message.web_view.is_height_valid) {
                 // Disable should_scroll after the message body has
                 // been loaded so we don't keep on scrolling later,
                 // like when the window has been resized.
@@ -306,7 +306,7 @@ public class ConversationListBox : Gtk.ListBox {
             // size of the body will be off, affecting the visibility
             // of emails further down the conversation.
             if (email_view.email.is_unread().is_certain() &&
-                conversation_message.is_loading_complete &&
+                conversation_message.web_view.is_height_valid &&
                 !email_view.is_manually_read) {
                  int body_top = 0;
                  int body_left = 0;
diff --git a/src/client/conversation-viewer/conversation-message.vala 
b/src/client/conversation-viewer/conversation-message.vala
index ca43686..344aa62 100644
--- a/src/client/conversation-viewer/conversation-message.vala
+++ b/src/client/conversation-viewer/conversation-message.vala
@@ -97,9 +97,6 @@ public class ConversationMessage : Gtk.Box {
     /** The specific RFC822 message displayed by this view. */
     public Geary.RFC822.Message message { get; private set; }
 
-    /** Specifies if the message body been been fully loaded. */
-    public bool is_loading_complete = false;
-
     /** Box containing the preview and full header widgets.  */
     [GtkChild]
     internal Gtk.Box summary_box;
@@ -406,11 +403,19 @@ public class ConversationMessage : Gtk.Box {
         }
 
         load_cancelled.cancelled.connect(() => { web_view.stop_loading(); });
-        this.web_view.notify["load-status"].connect((source, param) => {
-                if (this.web_view.load_status == WebKit.LoadStatus.FINISHED) {
-                    if (load_images) {
-                        show_images(false);
-                    }
+        // XXX Hook up unset_controllable_quotes() to size_allocate
+        // and check is_height_valid since we need to accurately know
+        // what the sizes of the quote and its container is to
+        // determine if it should be unhidden. However this means that
+        // when the user expands a hidden quote, this handler gets
+        // executed again and since the expanded quote will meet the
+        // criteria for being unset as controllable, that will
+        // happen. That's actually okay for now though, because if the
+        // user could collapse the quote again the space wouldn't be
+        // reclaimed, which is worse than this.
+        this.web_view.size_allocate.connect(() => {
+                if (this.web_view.load_status == WebKit.LoadStatus.FINISHED &&
+                    this.web_view.is_height_valid) {
                     WebKit.DOM.HTMLElement html = (
                         this.web_view.get_dom_document().document_element as
                         WebKit.DOM.HTMLElement
@@ -423,6 +428,13 @@ public class ConversationMessage : Gtk.Box {
                                     error.message);
                         }
                     }
+                }
+            });
+        this.web_view.notify["load-status"].connect((source, param) => {
+                if (this.web_view.load_status == WebKit.LoadStatus.FINISHED) {
+                    if (load_images) {
+                        show_images(false);
+                    }
                     Util.DOM.bind_event(
                         this.web_view, "html", "contextmenu",
                         (Callback) on_context_menu, this
@@ -439,10 +451,6 @@ public class ConversationMessage : Gtk.Box {
                         this.web_view, ".%s > .hider".printf(QUOTE_CONTAINER_CLASS),
                         "click",
                         (Callback) on_hide_quote_clicked, this);
-
-                    // XXX Not actually true since remote images will
-                    // still be loading.
-                    this.is_loading_complete = true;
                 }
             });
 
diff --git a/src/client/conversation-viewer/conversation-web-view.vala 
b/src/client/conversation-viewer/conversation-web-view.vala
index c20a9a9..72f2ae5 100644
--- a/src/client/conversation-viewer/conversation-web-view.vala
+++ b/src/client/conversation-viewer/conversation-web-view.vala
@@ -15,7 +15,7 @@ public class ConversationWebView : StylishWebView {
     private const string USER_CSS = "user-message.css";
 
     public string allow_prefix { get; private set; default = ""; }
-    public bool is_height_valid = false;
+    public bool is_height_valid { get; private set; default = false; }
 
     public signal void link_selected(string link);
 
@@ -86,28 +86,24 @@ public class ConversationWebView : StylishWebView {
         // warning in GTK 3.20-ish.
         base.get_preferred_height(out minimum_height, out natural_height);
 
-        int preferred_height = 0;
-        if (load_status == WebKit.LoadStatus.FINISHED) {
-            WebKit.DOM.Element html =
-                get_dom_document().get_document_element();
-            preferred_height = (int) html.offset_height;
-
-            // XXX Currently, for some messages the WebView will report
-            // very large offset heights, causing GDK and X allocation
-            // failures/warnings. If we get one, log it and limit it.  A
-            // value of ~22000 was crashing my xserver with a WebView
-            // width of around 745.
-            const int MAX = 15000;
-            this.is_height_valid = preferred_height > MAX;
-            if (this.is_height_valid) {
-                warning("WebView size reported as %lix%li, clamping height",
-                        html.offset_height,
-                        html.offset_width);
-                preferred_height = MAX;
+        WebKit.DOM.Element html = get_dom_document().get_document_element();
+        int offset_height = (int) html.offset_height;
+        int offset_width = (int) html.offset_width;
+
+        // If the offset_width is very small, the offset_height will
+        // likely be bogus, so just pretend we have no height for the
+        // moment. WebKitGTK seems to report an offset width of 1 in
+        // these cases.
+        if (offset_width > 1) {
+            // Avoid multiple notify signals?
+            if (!this.is_height_valid) {
+                this.is_height_valid = true;
             }
+        } else {
+            offset_height = 0;
         }
 
-        minimum_height = natural_height = preferred_height;
+        minimum_height = natural_height = offset_height;
     }
 
     // Overridden since we always what the view to be sized according