From 01a0f96b40345642b1100a583b13a2f67487f642 Mon Sep 17 00:00:00 2001 From: Michael Gratton Date: Sat, 17 Oct 2020 11:29:05 +1100 Subject: [PATCH 1/5] ui/components-web-view.js: Use ResizeObserver for watching pref height Rather than guessing when the height might change by using a number of different event listeners, use a ResizeObserver to get direct notifications of any changes to the HTML element's size. --- ui/components-web-view.js | 54 ++++----------------------------------- 1 file changed, 5 insertions(+), 49 deletions(-) diff --git a/ui/components-web-view.js b/ui/components-web-view.js index d0998a67..34bf0430 100644 --- a/ui/components-web-view.js +++ b/ui/components-web-view.js @@ -41,7 +41,12 @@ PageState.prototype = { } }); + this.heightObserver = new ResizeObserver((entries) => { + state.updatePreferredHeight(); + }); + document.addEventListener("DOMContentLoaded", function(e) { + state.heightObserver.observe(window.document.documentElement); state.loaded(); }); @@ -49,55 +54,6 @@ PageState.prototype = { 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. - let queueTimeout = null; - let queuePreferredHeightUpdate = function() { - if (queueTimeout != null) { - clearTimeout(queueTimeout); - } - queueTimeout = setTimeout( - function() { state.updatePreferredHeight(); }, 100 - ); - }; - - // Queues an update when the complete document is loaded. - // - // Note also that the delay introduced here by this last call - // to queuePreferredHeightUpdate when the complete document is - // loaded seems to be important to get an accurate idea of the - // final document size. - window.addEventListener("load", function(e) { - queuePreferredHeightUpdate(); - }, true); // load does not bubble - - // Queues updates for any STYLE, IMG and other loaded - // elements, hence handles resizing when the user later - // requests remote images loading. - document.addEventListener("load", function(e) { - queuePreferredHeightUpdate(); - }, true); // load does not bubble - - // Queues an update if the window changes size, e.g. if the - // user resized the window. Only trigger when the width has - // changed however since the height should only change as the - // body is being loaded. - let width = window.innerWidth; - window.addEventListener("resize", function(e) { - let currentWidth = window.innerWidth; - if (width != currentWidth) { - width = currentWidth; - queuePreferredHeightUpdate(); - } - }, false); // load does not bubble - - // Queues an update when a transition has completed, e.g. if the - // user resized the window - window.addEventListener("transitionend", function(e) { - queuePreferredHeightUpdate(); - }, false); // load does not bubble - this.testResult = null; }, getPreferredHeight: function() { From 0dce103f6b7d2195efe995f72bf1dc2f379e0824 Mon Sep 17 00:00:00 2001 From: Michael Gratton Date: Sat, 17 Oct 2020 13:54:39 +1100 Subject: [PATCH 2/5] ui/components-web-view.js: Use arrow functions for better this scoping --- ui/components-web-view.js | 22 ++++++++++------------ 1 file changed, 10 insertions(+), 12 deletions(-) diff --git a/ui/components-web-view.js b/ui/components-web-view.js index 34bf0430..9b026dcd 100644 --- a/ui/components-web-view.js +++ b/ui/components-web-view.js @@ -26,32 +26,30 @@ PageState.prototype = { 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 // the document when editing. let modifiedId = null; - this.bodyObserver = new MutationObserver(function(records) { + this.bodyObserver = new MutationObserver((records) => { if (modifiedId == null) { - modifiedId = window.setTimeout(function() { - state.documentModified(); - state.checkCommandStack(); + modifiedId = window.setTimeout(() => { + this.documentModified(); + this.checkCommandStack(); modifiedId = null; }, 1000); } }); this.heightObserver = new ResizeObserver((entries) => { - state.updatePreferredHeight(); + this.updatePreferredHeight(); }); - document.addEventListener("DOMContentLoaded", function(e) { - state.heightObserver.observe(window.document.documentElement); - state.loaded(); + document.addEventListener("DOMContentLoaded", (e) => { + this.heightObserver.observe(window.document.documentElement); + this.loaded(); }); - document.addEventListener("selectionchange", function(e) { - state.selectionChanged(); + document.addEventListener("selectionchange", (e) => { + this.selectionChanged(); }); this.testResult = null; From 40f97de745741fe54a8ece2ddd106cb356c8811d Mon Sep 17 00:00:00 2001 From: Michael Gratton Date: Sat, 17 Oct 2020 13:36:30 +1100 Subject: [PATCH 3/5] ui/conversation-web-view.css: Clean up HTML/BODY element lockdowns Use some more obvious CSS to ensure we can get an accurate idea of the content height for sizing the web view and that the body fits the web view's width. --- ui/conversation-web-view.css | 21 +++++++++++---------- 1 file changed, 11 insertions(+), 10 deletions(-) diff --git a/ui/conversation-web-view.css b/ui/conversation-web-view.css index d4786b4a..3c1929f2 100644 --- a/ui/conversation-web-view.css +++ b/ui/conversation-web-view.css @@ -1,5 +1,8 @@ /** * Style that is inserted into the message after it is loaded. + * + * Copyright © 2016 Software Freedom Conservancy Inc. + * Copyright © 2020 Michael Gratton */ /* @@ -14,17 +17,15 @@ html { color: black; background-color: white; - /* Trigger CSS 2.1 § 10.6.7 to get a shrink-wrapped height. */ - position: absolute !important; - top: 0 !important; - bottom: auto !important; - height: auto !important; + /* Width must always be defined by the viewport so content doesn't + overflow inside the WebView, height must always be defined by the + content so the WebView can be sized to fit exactly. */ + width: 100vw !important; + height: max-content !important; - /* Fix up the width after going to absolute positioning above. */ - width: 100%; - - /* Lock down the box just enough so we don't get an incrementally - expanding web view */ + /* Lock down the box sizing just enough so that the width and height + constraints above work as expected, and so the element's + scrollHeight is accurate. */ box-sizing: border-box !important; margin: 0 !important; border-width: 0 !important; From 0e783de5bfd9b2e740979f2eed58e60fa85bdfc0 Mon Sep 17 00:00:00 2001 From: Michael Gratton Date: Sat, 17 Oct 2020 13:38:16 +1100 Subject: [PATCH 4/5] ui/conversation-web-view.css: Work around oversized email body height Sometimes when loading an email body, the viewport for the web view will be set to 0 (when the web view is hidden or not yet laid out in the widget hierarchy?). When this happens, since the width of the body is specified as 100vw, the content width is reduced to the absolute minimum and hence the content height is stretched right out. Then, when the web view is displayed, the viewport width increases but the extra whitespace is never reclaimed (scrollHeight never goes down), so the height of the web view remains way too large, causing large amounts of whitespace at the end of the email message (i.e. #283). To work around this, set a min width for the HTML element so the initial height of the email body isn't too badly wrong. --- ui/conversation-web-view.css | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/ui/conversation-web-view.css b/ui/conversation-web-view.css index 3c1929f2..8b0ef421 100644 --- a/ui/conversation-web-view.css +++ b/ui/conversation-web-view.css @@ -23,6 +23,15 @@ html { width: 100vw !important; height: max-content !important; + /* Despite the fact that the width must always be defined by the + viewport, the viewport width will be 0 if the email is loaded before + its WebView is laid out in the widget hierarchy. As a workaround, to + prevent this causing the email being squished down to is minimum + width and hence being stretched right out in height, set a + reasonable minimum width. See + https://gitlab.gnome.org/GNOME/geary/-/issues/283 */ + min-width: 400px !important; + /* Lock down the box sizing just enough so that the width and height constraints above work as expected, and so the element's scrollHeight is accurate. */ From ec3057daf7fc8565960f913303d292833bc269cb Mon Sep 17 00:00:00 2001 From: Michael Gratton Date: Sat, 17 Oct 2020 13:45:23 +1100 Subject: [PATCH 5/5] ConversationWebView: Fix plain text emails sometimes being too wide Using `whitespace: pre-wrap` to format plain text email sometimes causes additional width to be allocated by the plain text blocks that then does not get used due to the constraints on the HTML element. The allocated space remains however and hence an un-needed horizontal scrollbar appears. Using `break-spaces` instead seems to help since it allows breaks after a space character, leading to the additional space not otherwise being allocated. --- src/engine/rfc822/rfc822-gmime-filter-blockquotes.vala | 2 +- test/engine/rfc822/rfc822-message-test.vala | 2 +- ui/composer-web-view.css | 2 +- ui/composer-web-view.js | 2 +- ui/conversation-web-view.css | 2 +- ui/conversation-web-view.js | 2 +- 6 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/engine/rfc822/rfc822-gmime-filter-blockquotes.vala b/src/engine/rfc822/rfc822-gmime-filter-blockquotes.vala index 8cf7563d..d2a941c0 100644 --- a/src/engine/rfc822/rfc822-gmime-filter-blockquotes.vala +++ b/src/engine/rfc822/rfc822-gmime-filter-blockquotes.vala @@ -59,7 +59,7 @@ private class Geary.RFC822.FilterBlockquotes : GMime.Filter { if (!initial_element) { // We set the style explicitly so it will be set in HTML emails. We also give it a // class so users can customize the style in the viewer. - insert_string("
", ref out_index); + insert_string("
", ref out_index); initial_element = true; } diff --git a/test/engine/rfc822/rfc822-message-test.vala b/test/engine/rfc822/rfc822-message-test.vala index b7697dda..57976cca 100644 --- a/test/engine/rfc822/rfc822-message-test.vala +++ b/test/engine/rfc822/rfc822-message-test.vala @@ -15,7 +15,7 @@ class Geary.RFC822.MessageTest : TestCase { private const string BASIC_MULTIPART_TNEF = "basic-multipart-tnef.eml"; private const string HTML_CONVERSION_TEMPLATE = - "
%s
"; + "
%s
"; private const string BASIC_PLAIN_BODY = """This is the first line. diff --git a/ui/composer-web-view.css b/ui/composer-web-view.css index 07ae6869..462d5876 100644 --- a/ui/composer-web-view.css +++ b/ui/composer-web-view.css @@ -60,6 +60,6 @@ blockquote { } pre { - white-space: pre-wrap; + white-space: break-spaces; margin: 0; } diff --git a/ui/composer-web-view.js b/ui/composer-web-view.js index 5ee4105e..bd010b6c 100644 --- a/ui/composer-web-view.js +++ b/ui/composer-web-view.js @@ -264,7 +264,7 @@ ComposerPageState.prototype = { }, tabOut: function() { document.execCommand( - "inserthtml", false, "\t" + "inserthtml", false, "\t" ); }, tabIn: function() { diff --git a/ui/conversation-web-view.css b/ui/conversation-web-view.css index 8b0ef421..d1da2eae 100644 --- a/ui/conversation-web-view.css +++ b/ui/conversation-web-view.css @@ -77,7 +77,7 @@ blockquote { } pre { - white-space: pre-wrap; + white-space: break-spaces; } /** diff --git a/ui/conversation-web-view.js b/ui/conversation-web-view.js index 1d730d47..7b3a1c89 100644 --- a/ui/conversation-web-view.js +++ b/ui/conversation-web-view.js @@ -213,7 +213,7 @@ ConversationPageState.prototype = { if (ConversationPageState.isDescendantOf( ancestor, "DIV", "plaintext", false)) { dummy.classList.add("plaintext"); - dummy.setAttribute("style", "white-space: pre-wrap;"); + dummy.setAttribute("style", "white-space: break-spaces;"); includeDummy = true; } dummy.appendChild(range.cloneContents());