[PATCH 0/3] Fix gnus-inhibit-images bug in Emacs 24

[PATCH 0/3] Fix gnus-inhibit-images bug in Emacs 24

[Austin Clements]

This series adds a test for the gnus-inhibit-images bug and then
provides one possile work-around.

Another possible approach is to advise mm-shr to load gnus-art.  That
might be a better approach, given that mm-shr is the only function
outside of gnus-art itself that references gnus-inhibit-images.

[PATCH 1/3] test: Clear test-ouput output file before running Emacs tests

[Austin Clements]

Most Emacs tests end with a call to (test-output), which saves the
buffer to a filed called OUTPUT.  Previously, if the test code failed
with an exception before this call, the test framework would then
compare against the OUTPUT file from the last Emacs test, resulting in
confusing diffs.

This requires one tweak to an emacs test that made two calls to
test_emacs and expected an OUTPUT file from the first call.  We simply
reverse the order of the test_emacs calls.
---
 test/emacs       |    4 ++--
 test/test-lib.sh |    8 ++++++++
 2 files changed, 10 insertions(+), 2 deletions(-)

diff --git a/test/emacs b/test/emacs
index 5d118b6..174a9ac 100755
--- a/test/emacs
+++ b/test/emacs
@@ -653,6 +653,8 @@ test_expect_equal "$(cat OUTPUT)" "thread:XXX"
 test_begin_subtest 'notmuch-show-advance-and-archive with invisible signature'
 message1='id:20091118010116.GC25380@dottiness.seas.harvard.edu'
 message2='id:1258491078-29658-1-git-send-email-dottedmag@dottedmag.net'
+test_emacs "(notmuch-show \"$message2\")
+	    (test-output \"EXPECTED\")"
 test_emacs "(notmuch-search \"$message1 or $message2\")
 	    (notmuch-test-wait)
 	    (notmuch-search-show-thread)
@@ -660,8 +662,6 @@ test_emacs "(notmuch-search \"$message1 or $message2\")
 	    (redisplay)
 	    (notmuch-show-advance-and-archive)
 	    (test-output)"
-test_emacs "(notmuch-show \"$message2\")
-	    (test-output \"EXPECTED\")"
 test_expect_equal_file OUTPUT EXPECTED
 
 test_begin_subtest "Refresh show buffer"
diff --git a/test/test-lib.sh b/test/test-lib.sh
index f34b1fb..7448b45 100644
--- a/test/test-lib.sh
+++ b/test/test-lib.sh
@@ -985,6 +985,14 @@ test_emacs () {
 		done
 	fi
 
+	# Clear test-output output file.  Most Emacs tests end with a
+	# call to (test-output).  If the test code fails with an
+	# exception before this call, the output file won't get
+	# updated.  Since we don't want to compare against an output
+	# file from another test, so start out with an empty file.
+	rm -f OUTPUT
+	touch OUTPUT
+
 	emacsclient --socket-name="$EMACS_SERVER" --eval "(progn $@)"
 }
 
-- 
1.7.10

[PATCH 2/3] test: Add a test for HTML email with inline images

[Austin Clements]

Currently this test passes in Emacs 23 but fails in Emacs 24 (at least
on some Linux distributions).
---
 test/emacs |   34 ++++++++++++++++++++++++++++++++++
 1 file changed, 34 insertions(+)

diff --git a/test/emacs b/test/emacs
index 174a9ac..1f84b91 100755
--- a/test/emacs
+++ b/test/emacs
@@ -749,4 +749,38 @@ counter=$(test_emacs \
 )
 test_expect_equal "$counter" 2
 
+
+test_begin_subtest "Rendering HTML mail with images"
+add_message '[subject]="HTML mail with images"' \
+    '[content-type]="multipart/related; boundary=abcd"' \
+    '[body]="--abcd
+Content-Type: text/html
+
+<img src="cid:330@goomoji.gmail">
+
+--abcd
+Content-Type: image/gif
+Content-Transfer-Encoding: base64
+Content-ID: <330@goomoji.gmail>
+
+R0lGODlhDAAMAKIFAF5LAP/zxAAAANyuAP/gaP///wAAAAAAACH5BAEAAAUALAAAAAAMAAwAAAMl
+WLPcGjDKFYi9lxKBOaGcF35DhWHamZUW0K4mAbiwWtuf0uxFAgA7
+--abcd--"'
+test_emacs "(notmuch-show \"id:${gen_msg_id}\")
+	    (test-output)"
+# Normalize output for Emacs 23 and Emacs 24
+sed -i 's/\[cid\]/*/' OUTPUT
+cat <<EOF >EXPECTED
+Notmuch Test Suite <test_suite@notmuchmail.org> (2001-01-05) (inbox)
+Subject: HTML mail with images
+To: Notmuch Test Suite <test_suite@notmuchmail.org>
+Date: Fri, 05 Jan 2001 15:43:57 +0000
+
+[ multipart/related ]
+[ text/html ]
+*
+EOF
+test_expect_equal_file OUTPUT EXPECTED
+
+
 test_done
-- 
1.7.10

Re: [PATCH 2/3] test: Add a test for HTML email with inline images

[Dmitry Kurochkin]

Hi Austin.

Austin Clements <amdragon@MIT.EDU> writes:

> Currently this test passes in Emacs 23 but fails in Emacs 24 (at least
> on some Linux distributions).

The test fails for me on Emacs 23.4.1 (Debian unstable):

 FAIL   Rendering HTML mail with images
	--- emacs.51.OUTPUT	2012-10-03 09:31:33.383529764 +0000
	+++ emacs.51.EXPECTED	2012-10-03 09:31:33.383529764 +0000
	@@ -6,4 +6,3 @@
	 [ multipart/related ]
	 [ text/html ]
	 *
	-

Did not look into details.

Regards,
  Dmitry

Re: [PATCH 2/3] test: Add a test for HTML email with inline images

[Austin Clements]

Quoth Dmitry Kurochkin on Oct 03 at  1:35 pm:
> Hi Austin.
> 
> Austin Clements <amdragon@MIT.EDU> writes:
> 
> > Currently this test passes in Emacs 23 but fails in Emacs 24 (at least
> > on some Linux distributions).
> 
> The test fails for me on Emacs 23.4.1 (Debian unstable):
> 
>  FAIL   Rendering HTML mail with images
> 	--- emacs.51.OUTPUT	2012-10-03 09:31:33.383529764 +0000
> 	+++ emacs.51.EXPECTED	2012-10-03 09:31:33.383529764 +0000
> 	@@ -6,4 +6,3 @@
> 	 [ multipart/related ]
> 	 [ text/html ]
> 	 *
> 	-
> 
> Did not look into details.

Yes.  This test is (in hindsight, unsurprisingly) sensitive to
whatever HTML renderer Emacs chooses.  It looks like you're probably
using html2text, which outputs nothing for an image.  Unfortunately,
none of the built-in renderers in Emacs 23 are aware of content
references, which makes this test rather pointless on Emacs 23 unless
we depend on an external renderer.

The best solution I can think of dynamically chooses shr on Emacs 24
(since that's really what we're trying to test) and gives up on Emacs
23 and forcibly selects html2text (test patch below).  Alternatively,
we could cycle through all of the available renderers, test everything
that we can, and just ignore everything that we can't run, though that
would make the test environment-sensitive.

diff --git a/test/emacs b/test/emacs
index 1f84b91..2ef78bf 100755
--- a/test/emacs
+++ b/test/emacs
@@ -756,7 +756,7 @@ add_message '[subject]="HTML mail with images"' \
     '[body]="--abcd
 Content-Type: text/html
 
-<img src="cid:330@goomoji.gmail">
+<img src="cid:330@goomoji.gmail"> smiley
 
 --abcd
 Content-Type: image/gif
@@ -766,10 +766,13 @@ Content-ID: <330@goomoji.gmail>
 R0lGODlhDAAMAKIFAF5LAP/zxAAAANyuAP/gaP///wAAAAAAACH5BAEAAAUALAAAAAAMAAwAAAMl
 WLPcGjDKFYi9lxKBOaGcF35DhWHamZUW0K4mAbiwWtuf0uxFAgA7
 --abcd--"'
-test_emacs "(notmuch-show \"id:${gen_msg_id}\")
+test_emacs "(let ((mm-text-html-renderer
+                  (if (assq 'shr mm-text-html-renderer-alist)
+                      'shr 'html2text)))
+             (notmuch-show \"id:${gen_msg_id}\"))
            (test-output)"
 # Normalize output for Emacs 23 and Emacs 24
-sed -i 's/\[cid\]/*/' OUTPUT
+sed -i 's/^ smiley/* smiley/' OUTPUT
 cat <<EOF >EXPECTED
 Notmuch Test Suite <test_suite@notmuchmail.org> (2001-01-05) (inbox)
 Subject: HTML mail with images
@@ -778,7 +781,7 @@ Date: Fri, 05 Jan 2001 15:43:57 +0000
 
 [ multipart/related ]
 [ text/html ]
-*
+* smiley
 EOF
 test_expect_equal_file OUTPUT EXPECTED
 

Re: [PATCH 2/3] test: Add a test for HTML email with inline images

[Dmitry Kurochkin]

Austin Clements <amdragon@MIT.EDU> writes:

> Quoth Dmitry Kurochkin on Oct 03 at  1:35 pm:
>> Hi Austin.
>> 
>> Austin Clements <amdragon@MIT.EDU> writes:
>> 
>> > Currently this test passes in Emacs 23 but fails in Emacs 24 (at least
>> > on some Linux distributions).
>> 
>> The test fails for me on Emacs 23.4.1 (Debian unstable):
>> 
>>  FAIL   Rendering HTML mail with images
>> 	--- emacs.51.OUTPUT	2012-10-03 09:31:33.383529764 +0000
>> 	+++ emacs.51.EXPECTED	2012-10-03 09:31:33.383529764 +0000
>> 	@@ -6,4 +6,3 @@
>> 	 [ multipart/related ]
>> 	 [ text/html ]
>> 	 *
>> 	-
>> 
>> Did not look into details.
>
> Yes.  This test is (in hindsight, unsurprisingly) sensitive to
> whatever HTML renderer Emacs chooses.  It looks like you're probably
> using html2text, which outputs nothing for an image.  Unfortunately,
> none of the built-in renderers in Emacs 23 are aware of content
> references, which makes this test rather pointless on Emacs 23 unless
> we depend on an external renderer.
>
> The best solution I can think of dynamically chooses shr on Emacs 24
> (since that's really what we're trying to test) and gives up on Emacs
> 23 and forcibly selects html2text (test patch below).  Alternatively,
> we could cycle through all of the available renderers, test everything
> that we can, and just ignore everything that we can't run, though that
> would make the test environment-sensitive.
>

Perhaps the test should be skipped if shr is not available, like we do
for missing binaries?

Regards,
  Dmitry

> diff --git a/test/emacs b/test/emacs
> index 1f84b91..2ef78bf 100755
> --- a/test/emacs
> +++ b/test/emacs
> @@ -756,7 +756,7 @@ add_message '[subject]="HTML mail with images"' \
>      '[body]="--abcd
>  Content-Type: text/html
>  
> -<img src="cid:330@goomoji.gmail">
> +<img src="cid:330@goomoji.gmail"> smiley
>  
>  --abcd
>  Content-Type: image/gif
> @@ -766,10 +766,13 @@ Content-ID: <330@goomoji.gmail>
>  R0lGODlhDAAMAKIFAF5LAP/zxAAAANyuAP/gaP///wAAAAAAACH5BAEAAAUALAAAAAAMAAwAAAMl
>  WLPcGjDKFYi9lxKBOaGcF35DhWHamZUW0K4mAbiwWtuf0uxFAgA7
>  --abcd--"'
> -test_emacs "(notmuch-show \"id:${gen_msg_id}\")
> +test_emacs "(let ((mm-text-html-renderer
> +                  (if (assq 'shr mm-text-html-renderer-alist)
> +                      'shr 'html2text)))
> +             (notmuch-show \"id:${gen_msg_id}\"))
>             (test-output)"
>  # Normalize output for Emacs 23 and Emacs 24
> -sed -i 's/\[cid\]/*/' OUTPUT
> +sed -i 's/^ smiley/* smiley/' OUTPUT
>  cat <<EOF >EXPECTED
>  Notmuch Test Suite <test_suite@notmuchmail.org> (2001-01-05) (inbox)
>  Subject: HTML mail with images
> @@ -778,7 +781,7 @@ Date: Fri, 05 Jan 2001 15:43:57 +0000
>  
>  [ multipart/related ]
>  [ text/html ]
> -*
> +* smiley
>  EOF
>  test_expect_equal_file OUTPUT EXPECTED
>  

[PATCH 3/3] emacs: Work around gnus-inhibit-images bug in mm-shr

[Austin Clements]

Emacs 24's mm-shr HTML email renderer fails to load gnus-art before
referencing gnus-inhibit-images, resulting in a void-variable error
when notmuch attempts to render an HTML email with inline images.
This works around this bug by providing a definition for
gnus-inhibit-images.

This fixes the "Rendering HTML mail with images" test for Emacs 24.
---
 emacs/notmuch-lib.el |   24 ++++++++++++++++++++++++
 1 file changed, 24 insertions(+)

diff --git a/emacs/notmuch-lib.el b/emacs/notmuch-lib.el
index 20d990d..c74035c 100644
--- a/emacs/notmuch-lib.el
+++ b/emacs/notmuch-lib.el
@@ -254,6 +254,30 @@ the given type."
   (or (plist-get part :content)
       (notmuch-get-bodypart-internal (notmuch-id-to-query (plist-get msg :id)) nth process-crypto)))
 
+;; Workaround: The call to `mm-display-part' below triggers a bug in
+;; Emacs 24 if it attempts to use the shr renderer to display an HTML
+;; part with images in it (demonstrated in 24.1 and 24.2 on Debian and
+;; Fedora 17, though unreproducable in other configurations).
+;; `mm-shr' references the variable `gnus-inhibit-images' without
+;; first loading gnus-art, which defines it, resulting in a
+;; void-variable error.  Unfortunately, gnus-art and its dependencies
+;; are very large and often not needed, making it undesirable to
+;; simply load gnus-art here.  Instead, we directly define the
+;; variable here.  We use defcustom instead of defvar so that it
+;; interacts properly with user customizations of the variable (if the
+;; custom value has already been loaded, only defcustom will make it
+;; visible).  There is a danger of overriding the documentation string
+;; if gnus-art is already loaded, so we don't do anything if the
+;; variable is already defined.  If the user attempts to customize the
+;; variable, we instruct custom to load gnus-art before displaying the
+;; widget, which will override this definition with the proper
+;; documentation string, groups, etc.
+(if (and (>= emacs-major-version 24)
+         (not (boundp 'gnus-inhibit-images)))
+    (defcustom gnus-inhibit-images nil
+      "Documentation will be available after loading gnus-art."
+      :load "gnus-art"))
+
 (defun notmuch-mm-display-part-inline (msg part nth content-type process-crypto)
   "Use the mm-decode/mm-view functions to display a part in the
 current buffer, if possible."
-- 
1.7.10

[PATCH] emacs: Work around gnus-inhibit-images bug in mm-shr

[Austin Clements]

Emacs 24's mm-shr HTML email renderer fails to load gnus-art before
referencing gnus-inhibit-images, resulting in a void-variable error
when notmuch attempts to render an HTML email with inline images.
This works around this bug by advising mm-shr to load gnus-art.

mm-shr is the only function outside of gnus-art itself that references
gnus-inhibit-images, so this workaround should be correct.  If this
ever changes, hopefully they will have fixed this bug upstream first.

This fixes the "Rendering HTML mail with images" test for Emacs 24.
---

This is an alternate fix to the one provided in patch 3/3.

 emacs/notmuch-lib.el |   13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/emacs/notmuch-lib.el b/emacs/notmuch-lib.el
index 20d990d..69867ad 100644
--- a/emacs/notmuch-lib.el
+++ b/emacs/notmuch-lib.el
@@ -254,6 +254,19 @@ the given type."
   (or (plist-get part :content)
       (notmuch-get-bodypart-internal (notmuch-id-to-query (plist-get msg :id)) nth process-crypto)))
 
+;; Workaround: The call to `mm-display-part' below triggers a bug in
+;; Emacs 24 if it attempts to use the shr renderer to display an HTML
+;; part with images in it (demonstrated in 24.1 and 24.2 on Debian and
+;; Fedora 17, though unreproducable in other configurations).
+;; `mm-shr' references the variable `gnus-inhibit-images' without
+;; first loading gnus-art, which defines it, resulting in a
+;; void-variable error.  Hence, we advise `mm-shr' to ensure gnus-art
+;; is loaded.
+(if (>= emacs-major-version 24)
+    (defadvice mm-shr (before load-gnus-arts activate)
+      (require 'gnus-art nil t)
+      (ad-disable-advice 'mm-shr 'before 'load-gnus-arts)))
+
 (defun notmuch-mm-display-part-inline (msg part nth content-type process-crypto)
   "Use the mm-decode/mm-view functions to display a part in the
 current buffer, if possible."
-- 
1.7.10

Re: [PATCH 0/3] Fix gnus-inhibit-images bug in Emacs 24

[Tomi Ollila]

On Sat, Sep 29 2012, Austin Clements <amdragon@MIT.EDU> wrote:

> This series adds a test for the gnus-inhibit-images bug and then
> provides one possile work-around.
>
> Another possible approach is to advise mm-shr to load gnus-art.  That
> might be a better approach, given that mm-shr is the only function
> outside of gnus-art itself that references gnus-inhibit-images.

LGTM. My vote goes for patches (i.e. I like the advice approach).

id:"1348941314-8377-2-git-send-email-amdragon@mit.edu"
id:"1348941314-8377-3-git-send-email-amdragon@mit.edu"
id:"1348941823-15516-1-git-send-email-amdragon@mit.edu"

Tomi


>
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch

Re: [PATCH 0/3] Fix gnus-inhibit-images bug in Emacs 24

[David Bremner]

Tomi Ollila <tomi.ollila@iki.fi> writes:

> LGTM. My vote goes for patches (i.e. I like the advice approach).
>
> id:"1348941314-8377-2-git-send-email-amdragon@mit.edu"
> id:"1348941314-8377-3-git-send-email-amdragon@mit.edu"
> id:"1348941823-15516-1-git-send-email-amdragon@mit.edu"

I pushed this set,

d