* [PATCH 0/2] Bug and potential fix: forwarded messages
@ 2012-02-04 0:32 Adam Wolfe Gordon
2012-02-04 0:32 ` [PATCH 1/2] test: Add broken test for showing " Adam Wolfe Gordon
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Adam Wolfe Gordon @ 2012-02-04 0:32 UTC (permalink / raw)
To: notmuch
Hi everyone,
I encountered today a bug that I remember being mentioned on IRC where
some messages are shown in emacs as completely empty, even though their
content is present and simple. For me, the bug shows up with messages that
have been forwarded by an Outlook user who didn't add any content to the top
of the message. The first line of the message is "-----Original Message-----",
and the rest is the forwarded text. Notmuch tries to collapse the forwarded
content into a button, but fails mysteriously.
The first patch in this series adds a test (marked as broken) that demonstrates
the bug. I think this should be pushed regardless of whether my solution is
the right one. My solution is to check whether we're collapsing the entire
message before doing so, and avoid turning it into a button if that's the case.
I think this is a desirable behavior, since if someone has forwarded a message
without adding anything the user probably wants to read that message. But,
I'll admit that I didn't figure out the real cause of the problem, and I
would be happy to hear other suggestions.
It also occurs to me that this might indicate a bigger problem with how
notmuch-wash.el handles messages starting with "-----Original Message-----".
Notmuch seems to assume that this indicates the rest of the message is quoted
stuff that's been top-posted on. In my office this isn't necessarily the case,
since Outlook produces that line at the top of every reply, and it's up to
the user whether to top-post or not (and not everyone does).
I'll have to experiment a bit more to verify whether a problem (i.e. whether
Notmuch hides inappropriate things when someone replies inline with Outlook).
Someone please correct me if I'm missing something in how the code operates.
Adam Wolfe Gordon (2):
test: Add broken test for showing forwarded messages
emacs: Fix broken showing of forwarded messages.
emacs/notmuch-wash.el | 5 +++--
test/emacs | 32 ++++++++++++++++++++++++++++++++
2 files changed, 35 insertions(+), 2 deletions(-)
--
1.7.5.4
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 1/2] test: Add broken test for showing forwarded messages
2012-02-04 0:32 [PATCH 0/2] Bug and potential fix: forwarded messages Adam Wolfe Gordon
@ 2012-02-04 0:32 ` Adam Wolfe Gordon
2012-02-04 0:32 ` [PATCH 2/2] emacs: Fix broken showing of " Adam Wolfe Gordon
2012-02-04 6:01 ` [PATCH 0/2] Bug and potential fix: " Adam Wolfe Gordon
2 siblings, 0 replies; 6+ messages in thread
From: Adam Wolfe Gordon @ 2012-02-04 0:32 UTC (permalink / raw)
To: notmuch
Messages are sometimes forwarded with no new text in them, such
that the entire message looks like a top-posted reply with no reply
text. Showing such a message in emacs results in no message body
or button being inserted, so the buffer is empty except for headers.
---
test/emacs | 33 +++++++++++++++++++++++++++++++++
1 files changed, 33 insertions(+), 0 deletions(-)
diff --git a/test/emacs b/test/emacs
index 8ca4c8a..b8b13a7 100755
--- a/test/emacs
+++ b/test/emacs
@@ -349,6 +349,39 @@ Thanks for the advice! I will be sure to put it to good use.
[ 9-line hidden original message. Click/Enter to show. ]" > EXPECTED
test_expect_equal_file OUTPUT EXPECTED
+test_begin_subtest "Non-hiding of forwarded message with no new text"
+test_subtest_known_broken
+add_message [id]=forwarded-top-post \
+ '[subject]="FW: The problem with top-posting"' \
+ '[body]="
+
+-----Original Message-----
+From: Top Poster <top@poster.com>
+To: Notmuch Test Suite <test_suite@notmuchmai.org>
+Sent: Fri, 05 Jan 2001 15:43:57 +0000
+Subject: FW: The problem with top-posting
+
+Q: Why is top-posting such a bad thing?
+A: Top-posting.
+Q: What is the most annoying thing in e-mail?"'
+test_emacs "(notmuch-show \"FW: The problem with top-posting\")
+ (test-visible-output)"
+echo "Notmuch Test Suite <test_suite@notmuchmail.org> (2001-01-05) (inbox)
+Subject: FW: The problem with top-posting
+To: Notmuch Test Suite <test_suite@notmuchmail.org>
+Date: Fri, 05 Jan 2001 15:43:57 +0000
+
+-----Original Message-----
+From: Top Poster <top@poster.com>
+To: Notmuch Test Suite <test_suite@notmuchmai.org>
+Sent: Fri, 05 Jan 2001 15:43:57 +0000
+Subject: FW: The problem with top-posting
+
+Q: Why is top-posting such a bad thing?
+A: Top-posting.
+Q: What is the most annoying thing in e-mail?" > EXPECTED
+test_expect_equal_file OUTPUT EXPECTED
+
test_begin_subtest "Hiding message in notmuch-show view"
test_emacs '(notmuch-show "id:f35dbb950911171438k5df6eb56k77b6c0944e2e79ae@mail.gmail.com")
(notmuch-show-toggle-message)
--
1.7.5.4
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 2/2] emacs: Fix broken showing of forwarded messages.
2012-02-04 0:32 [PATCH 0/2] Bug and potential fix: forwarded messages Adam Wolfe Gordon
2012-02-04 0:32 ` [PATCH 1/2] test: Add broken test for showing " Adam Wolfe Gordon
@ 2012-02-04 0:32 ` Adam Wolfe Gordon
2012-02-04 6:01 ` [PATCH 0/2] Bug and potential fix: " Adam Wolfe Gordon
2 siblings, 0 replies; 6+ messages in thread
From: Adam Wolfe Gordon @ 2012-02-04 0:32 UTC (permalink / raw)
To: notmuch
Fix the bug in showing forwarded messages with no new text by not
hiding "original" messages if they are the entire message.
---
emacs/notmuch-wash.el | 5 +++--
test/emacs | 1 -
2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/emacs/notmuch-wash.el b/emacs/notmuch-wash.el
index 67143e5..2fe9e5d 100644
--- a/emacs/notmuch-wash.el
+++ b/emacs/notmuch-wash.el
@@ -176,8 +176,9 @@ insert before the button, probably for indentation."
(let* ((msg-start (match-beginning 0))
(msg-end (point-max))
(msg-lines (count-lines msg-start msg-end)))
- (notmuch-wash-region-to-button
- msg msg-start msg-end "original" "\n")))
+ (when (< msg-lines (count-lines (point-min) (point-max)))
+ (notmuch-wash-region-to-button
+ msg msg-start msg-end "original" "\n"))))
(while (and (< (point) (point-max))
(re-search-forward notmuch-wash-citation-regexp nil t))
(let* ((cite-start (match-beginning 0))
diff --git a/test/emacs b/test/emacs
index b8b13a7..52683dd 100755
--- a/test/emacs
+++ b/test/emacs
@@ -350,7 +350,6 @@ Thanks for the advice! I will be sure to put it to good use.
test_expect_equal_file OUTPUT EXPECTED
test_begin_subtest "Non-hiding of forwarded message with no new text"
-test_subtest_known_broken
add_message [id]=forwarded-top-post \
'[subject]="FW: The problem with top-posting"' \
'[body]="
--
1.7.5.4
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 0/2] Bug and potential fix: forwarded messages
2012-02-04 0:32 [PATCH 0/2] Bug and potential fix: forwarded messages Adam Wolfe Gordon
2012-02-04 0:32 ` [PATCH 1/2] test: Add broken test for showing " Adam Wolfe Gordon
2012-02-04 0:32 ` [PATCH 2/2] emacs: Fix broken showing of " Adam Wolfe Gordon
@ 2012-02-04 6:01 ` Adam Wolfe Gordon
2012-02-04 6:10 ` Dmitry Kurochkin
2 siblings, 1 reply; 6+ messages in thread
From: Adam Wolfe Gordon @ 2012-02-04 6:01 UTC (permalink / raw)
To: notmuch
Oh, and I just noticed that Dmitry has already fixed this, probably in
a better way [1] (though I maintain that there still may be a problem
with the approach in general). I clearly haven't been following the
list closely enough this week.
[1] id:"1327926286-16680-1-git-send-email-dmitry.kurochkin@gmail.com"
On Fri, Feb 3, 2012 at 17:32, Adam Wolfe Gordon <awg+notmuch@xvx.ca> wrote:
> Hi everyone,
>
> I encountered today a bug that I remember being mentioned on IRC where
> some messages are shown in emacs as completely empty, even though their
> content is present and simple. For me, the bug shows up with messages that
> have been forwarded by an Outlook user who didn't add any content to the top
> of the message. The first line of the message is "-----Original Message-----",
> and the rest is the forwarded text. Notmuch tries to collapse the forwarded
> content into a button, but fails mysteriously.
>
> The first patch in this series adds a test (marked as broken) that demonstrates
> the bug. I think this should be pushed regardless of whether my solution is
> the right one. My solution is to check whether we're collapsing the entire
> message before doing so, and avoid turning it into a button if that's the case.
> I think this is a desirable behavior, since if someone has forwarded a message
> without adding anything the user probably wants to read that message. But,
> I'll admit that I didn't figure out the real cause of the problem, and I
> would be happy to hear other suggestions.
>
> It also occurs to me that this might indicate a bigger problem with how
> notmuch-wash.el handles messages starting with "-----Original Message-----".
> Notmuch seems to assume that this indicates the rest of the message is quoted
> stuff that's been top-posted on. In my office this isn't necessarily the case,
> since Outlook produces that line at the top of every reply, and it's up to
> the user whether to top-post or not (and not everyone does).
>
> I'll have to experiment a bit more to verify whether a problem (i.e. whether
> Notmuch hides inappropriate things when someone replies inline with Outlook).
> Someone please correct me if I'm missing something in how the code operates.
>
> Adam Wolfe Gordon (2):
> test: Add broken test for showing forwarded messages
> emacs: Fix broken showing of forwarded messages.
>
> emacs/notmuch-wash.el | 5 +++--
> test/emacs | 32 ++++++++++++++++++++++++++++++++
> 2 files changed, 35 insertions(+), 2 deletions(-)
>
> --
> 1.7.5.4
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 0/2] Bug and potential fix: forwarded messages
2012-02-04 6:01 ` [PATCH 0/2] Bug and potential fix: " Adam Wolfe Gordon
@ 2012-02-04 6:10 ` Dmitry Kurochkin
2012-02-04 16:51 ` Adam Wolfe Gordon
0 siblings, 1 reply; 6+ messages in thread
From: Dmitry Kurochkin @ 2012-02-04 6:10 UTC (permalink / raw)
To: Adam Wolfe Gordon, notmuch
On Fri, 3 Feb 2012 23:01:23 -0700, Adam Wolfe Gordon <awg+notmuch@xvx.ca> wrote:
> Oh, and I just noticed that Dmitry has already fixed this, probably in
> a better way [1] (though I maintain that there still may be a problem
> with the approach in general). I clearly haven't been following the
> list closely enough this week.
>
Yep, creating buttons for regions at the beginning of message is fixed.
But I like the idea of not hiding the message if there is no text.
Though, I think we should create the button, just not hide the text
initially.
This change would conflict with my fix, so we should probably wait until
it is applied before working on this.
Regards,
Dmitry
> [1] id:"1327926286-16680-1-git-send-email-dmitry.kurochkin@gmail.com"
>
> On Fri, Feb 3, 2012 at 17:32, Adam Wolfe Gordon <awg+notmuch@xvx.ca> wrote:
> > Hi everyone,
> >
> > I encountered today a bug that I remember being mentioned on IRC where
> > some messages are shown in emacs as completely empty, even though their
> > content is present and simple. For me, the bug shows up with messages that
> > have been forwarded by an Outlook user who didn't add any content to the top
> > of the message. The first line of the message is "-----Original Message-----",
> > and the rest is the forwarded text. Notmuch tries to collapse the forwarded
> > content into a button, but fails mysteriously.
> >
> > The first patch in this series adds a test (marked as broken) that demonstrates
> > the bug. I think this should be pushed regardless of whether my solution is
> > the right one. My solution is to check whether we're collapsing the entire
> > message before doing so, and avoid turning it into a button if that's the case.
> > I think this is a desirable behavior, since if someone has forwarded a message
> > without adding anything the user probably wants to read that message. But,
> > I'll admit that I didn't figure out the real cause of the problem, and I
> > would be happy to hear other suggestions.
> >
> > It also occurs to me that this might indicate a bigger problem with how
> > notmuch-wash.el handles messages starting with "-----Original Message-----".
> > Notmuch seems to assume that this indicates the rest of the message is quoted
> > stuff that's been top-posted on. In my office this isn't necessarily the case,
> > since Outlook produces that line at the top of every reply, and it's up to
> > the user whether to top-post or not (and not everyone does).
> >
> > I'll have to experiment a bit more to verify whether a problem (i.e. whether
> > Notmuch hides inappropriate things when someone replies inline with Outlook).
> > Someone please correct me if I'm missing something in how the code operates.
> >
> > Adam Wolfe Gordon (2):
> > test: Add broken test for showing forwarded messages
> > emacs: Fix broken showing of forwarded messages.
> >
> > emacs/notmuch-wash.el | 5 +++--
> > test/emacs | 32 ++++++++++++++++++++++++++++++++
> > 2 files changed, 35 insertions(+), 2 deletions(-)
> >
> > --
> > 1.7.5.4
> >
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 0/2] Bug and potential fix: forwarded messages
2012-02-04 6:10 ` Dmitry Kurochkin
@ 2012-02-04 16:51 ` Adam Wolfe Gordon
0 siblings, 0 replies; 6+ messages in thread
From: Adam Wolfe Gordon @ 2012-02-04 16:51 UTC (permalink / raw)
To: Dmitry Kurochkin; +Cc: notmuch
On Fri, Feb 3, 2012 at 23:10, Dmitry Kurochkin
<dmitry.kurochkin@gmail.com> wrote:
> Yep, creating buttons for regions at the beginning of message is fixed.
> But I like the idea of not hiding the message if there is no text.
> Though, I think we should create the button, just not hide the text
> initially.
>
> This change would conflict with my fix, so we should probably wait until
> it is applied before working on this.
Yes, this sounds like a good idea. I'll wait until your fix is in
before giving it more thought.
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2012-02-04 16:52 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-02-04 0:32 [PATCH 0/2] Bug and potential fix: forwarded messages Adam Wolfe Gordon
2012-02-04 0:32 ` [PATCH 1/2] test: Add broken test for showing " Adam Wolfe Gordon
2012-02-04 0:32 ` [PATCH 2/2] emacs: Fix broken showing of " Adam Wolfe Gordon
2012-02-04 6:01 ` [PATCH 0/2] Bug and potential fix: " Adam Wolfe Gordon
2012-02-04 6:10 ` Dmitry Kurochkin
2012-02-04 16:51 ` Adam Wolfe Gordon
Code repositories for project(s) associated with this public inbox
https://yhetil.org/notmuch.git/
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).