From: Mark Walters <markwalters1009@gmail.com>
To: Austin Clements <amdragon@MIT.EDU>, notmuch@notmuchmail.org
Subject: Re: [PATCH v2] emacs: show: lazy part bugfix
In-Reply-To: <20130904161628.GC1426@mit.edu>
References: <1377246875-7784-1-git-send-email-markwalters1009@gmail.com>
	<1378279835-28288-1-git-send-email-markwalters1009@gmail.com>
	<20130904145639.GB1426@mit.edu>
	<87hae07fhs.fsf@servo.finestructure.net>
	<20130904161628.GC1426@mit.edu>
User-Agent: Notmuch/0.15.2+269~g01f5508 (http://notmuchmail.org) Emacs/23.4.1
	(x86_64-pc-linux-gnu)
Date: Thu, 05 Sep 2013 19:46:37 +0100
Message-ID: <87txhz14z6.fsf@qmul.ac.uk>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: quoted-printable
Precedence: list


Thanks to Austin's prodding and debugging and pointing out contradictory
evidence I have looked into this bug in more detail and think I have got
the actual cause this time.

Having chased it all down I think there is a one line fix. Since I had
already written the rest of the email when I discovered this I include
the diagnosis below but most people will probably want to skip to the
patch at the end.

---

The problem only occurs for message of this sort of form
   =E2=94=94=E2=94=AC=E2=95=B4multipart/alternative 896783 bytes
    =E2=94=9C=E2=94=80=E2=95=B4text/plain 379 bytes
    =E2=94=94=E2=94=AC=E2=95=B4multipart/related 892556 bytes
     =E2=94=9C=E2=94=80=E2=95=B4text/html 1236 bytes
     =E2=94=94=E2=94=80=E2=95=B4image/jpeg inline [photo.JPG] 890841 bytes

The important features are that initially the multipart/related is
hidden, and that when the multipart/related part is shown the child
image/jpeg is hidden (that is what happens in Istvan's patch and could
also happen with the inner part also being multipart/alternative but
not, I think, if the inner part were multipart/mixed).

Now the code for hiding parts starts with the normal button and then
toggles the button (this is to make sure the button gets the correct
label text). When doing so this code looks at the first character of the
button, stores the text-properties, replaces the button with the toggled
button, puts the text-properties back. Since insert does not put in text
properties we do need this save restore.

In the normal case where a hidden part is inserted it is on a newline
where the current text-properties are nil. Thus the save/restore of text
properties does nothing in this case; after this is done the correct
text-properties are applied to the whole part (with the proviso that
:notmuch-part does not over-ride subpart's :notmuch-part property)

In the problem case above we have a line with=20

[multipart/related (hidden)]

The whole of this line including the \n has the multipart-related
:notmuch-part

Now when we do the lazy insertion we toggle this button (which behaves
correctly), go to the end of the button insert a \n and then insert the
child parts.

At this point we have

[multipart/related]\n
\n

the [multipart/related] has the multipart/related :notmuch-part info the
first \n does not (as we just inserted) and the second \n does because
it is the original \n

Now if we insert a part button for a hidden part we insert the button
and toggle it. Finally we get the problem: this toggle saves and
restores (to the whole button) the text-properties from *point* which is
the \n so includes the multipart/related
:notmuch-part text-property

If we save and restore from the start of the button we solve all of the
problems (I think)

Best wishes=20

Mark


---
 emacs/notmuch-show.el |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
index 20844f0..0267574 100644
--- a/emacs/notmuch-show.el
+++ b/emacs/notmuch-show.el
@@ -503,7 +503,7 @@ message at DEPTH in the current thread."
 	     (new-start (button-start button))
 	     (button-label (button-get button :base-label))
 	     (old-point (point))
-	     (properties (text-properties-at (point)))
+	     (properties (text-properties-at (button-start button)))
 	     (inhibit-read-only t))
 	;; Toggle the button itself.
 	(button-put button :notmuch-part-hidden (not show))
--=20
1.7.9.1