Closed Bug 102848 Opened 23 years ago Closed 22 years ago

Remove (was: ... in Subject line

Categories

(MailNews Core :: Composition, enhancement, P2)

Product:
MailNews Core
Component:
Composition
Type:
enhancement

Tracking

(Not tracked)

Status:
VERIFIED FIXED
Milestone:
mozilla1.3beta

People

(Reporter: 3.14, Assigned: Biesinger)

References

(Blocks 1 open bug,
URL
)

Details

Attachments

(1 file, 2 obsolete files)

patch v3
3.44 KB, patch
bugzilla
: review+
Bienvenu
: superreview+
Details | Diff | Splinter Review 
Mozilla/5.0 (X11; U; Linux i686; en-US; rv:0.9.4) Gecko/20010913

Suggestion for improvement: Remove the part starting with blank (was: from the
subject in replies/followups.

pi
reporter,
please explain with some steps and an example. 
Assume you have an discussion about the Netscape browser with subject
"Netscape". Now the topic changes toward Mozilla. So -- according to son-of-1036
the subject is changed to "Mozialla (was: Netsacpe)". It is best practice to
have follow-ups with Subject "Re: Mozilla" only, i.e., deleting the (was:-part.
The same applies to e-mail, especially for mailing lists.
Reporter,
Do you mean when you are replying to emails using netscape client you get
subject as was: instead of Re:?

Can you give me a test case to be able to reproduce this.  Because I have see
Re: as subject when you are replying to the emails.  
No, i mean that in my example with subject "Mozialla (was: Netsacpe)" I get "Re:
Mozialla (was: Netsacpe)", where best practice would be to get just "Re: Mozialla".

pi
*** This bug has been confirmed by popular vote. ***
Status: UNCONFIRMED → NEW
Ever confirmed: true
This handling of subject lines is common in many professional newsreaders,
especially in *ix, such as slrn.
A thread in a newsgroup starts with the following subject:

subject: This is a test

People reply to it...

subject: Re: This is a test

Someone changes the subject line to something more approriate...

subject: What a nice test (was: This is a test)

When replying with a smart newsreader, the "was"-part will be cut off, leaving
the following subject line:

subject: Re: What a nice test

It would be nice if Mozilla had this feature.
Target Milestone: --- → Future
Status: NEW → ASSIGNED
OS: Linux → All
Hardware: PC → All
Probably this bug has an easy fix. Using Perl-notation applied to the not yet
encoded subject I'd suggest:
s/ \(was: (?!.* \(was: ).*//s
In words: Find the last string " (was: " in the subject. If exists, remove it
and everything which follows.

pi
Also see example in section 5.4.1. of the usefor draft at the URL above.

pi
URL: http://www.landfield.com/usefor/draft...
Attached patch patch (obsolete) — — Splinter Review 

    
    

    
  
taking bug
Assignee: ducarroz → cbiesinger
Status: ASSIGNED → NEW
Status: NEW → ASSIGNED
Priority: -- → P2
Target Milestone: Future → mozilla1.2beta

    
    

    
  
Comment on attachment 101762 [details] [diff] [review]
patch

Looks good. R=ducarroz.

I wonder if you should check if the subject end with a close parenthesis 
before stripping the old subject!

        Attachment #101762 -
        Flags: review+

    
    

    
  
hmm. I don't think anyone would have a subject where stripping (was: ... is not
desired, even if a closing parenthesis is missing... so it is, imho, not worth
the extra effort.

but sure, if you or the super-reviewer disagree, I can make that change.

    
    

    
  
I agree with the last comment. Sometimes the closing parenthesis is missing.
Also it is not clear how to look for it, since there might be more than one (if
the previous subject already included one).

pi

    
    

    
  
A few questions: 

what if someone legitimately had "(was:" in their subject? (like the bugzilla
messages generated by this bug :-) )

is this best practice only for newsreaders? In which case, should we only do
this for news?

 Also, my reading of the rfc says that the newsreader can tell if the (was: was
added : " Software can always recognize such changes from the
References-header." This implies to me that we could look at the references to
find the original msg headers and see if original subject really was the subject
after "(was:"

    
    

    
  
You are right, there is a small risk which cannot be avoided, similiar:
Subject: Re: -- what does it mean
Subject: cmsg what?
Yes, the RfC explicitely does not allow these two examples, for (was: this does
not work. I have to say, that I have never seen this problem happen in real
life, though. To avoid it, the user can use quotes.

This is best practice for news, but is also used a lot in mailing lists. So I'd
leave it in both.

Your final suggestion about the references seems to complicated. Also note it
can fail, if the message from the references is no longer available.

I hope this settles your questions.

pi

    
    

    
  
>what if someone legitimately had "(was:" in their subject?

hm. then it would be stripped off too. however, can you come up with an example?
(I can't)

> (like the bugzilla
>messages generated by this bug :-) )

bad example, because you don't reply to bugzilla mails :)

    
    

    
  
David, can you sr this? So we could get that in for 1.2b?

pi
Whiteboard: fix in hand, needs sr=, needs a=

    
    

    
  
Patch WFM: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.2b) Gecko/2002101815 (+
patches 101274, 101762, 103210)

pi
Keywords: mozilla1.2

    
    

    
  
Fixing this bug will _break_ requiered news client behaviour as defined by The
Good Net-Keeping Seal of Approval (GKNSA
<URL:http://www.xs4all.nl/~js/gnksa/gnksa.txt>), rule 5b. Since Mozilla is
supposed to be standards compiliant I suggest WONTFIX.

    
    

    
  
If this really breaks "requiered news client behaviour" then make it a (hidden?)
pref with default=off. So the (advanced) user can choose whether to break the
requiered news client behaviour or not.

    
    

    
  
Re comment 19: I don't see this in contrast to GNKSA. If you look at the
rationale there you see why it should not be changed. Usefor draft (URL above)
explicitely gives the example of removing the (was: part. It is the new subject
which remains in full (as this fix correctly implements it). Can you, Torben,
point to any GNKSA evaluation which failed due to the removal of the (was: part?

pi

    
    

    
  
MacSOUP 2.4.4 fails the GNKSA evalution because of this, from
<URL:http://www.xs4all.nl/~js/gnksa/Evaluations/macsoup-2.4.4.txt >:

Regarding the strict GNKSA requirements (MUSTs), MacSOUP 2.4.4
FAILS on the following points:

    5b   Does not preserve entire original subject

Additional Comments:

5b -- MacSOUP deletes a portion of the Subject in followups under some
      circumstances; namely it will delete "(was:" and anything that 
      follows it.  The user could add this back in by hand, but there is
      no other way to change this behavior.

This evaluation is older than the draft referred to here, but I cannot find any
thing in the draft saying that the removal of (was: should or can be done by the
software.

    
    

    
  
Re: Comment #22 From Torben 2002-11-04 02:32

> 5b -- MacSOUP deletes a portion of the Subject in followups under some
>      circumstances; namely it will delete "(was:" and anything that 
>      follows it.  The user could add this back in by hand, but there is
>      no other way to change this behavior.

That's quite a dumb rule, frankly spoken. What happens with a very, very long
thread, starting with a totally wrong subject "Problem", continuing with "MIME
Type Issue (was: Problem)", then "should the browser download text/plain files
in some cases? (was: MIME Type Issue (was: Porblem))"? Now, a troll jumps in:
"you all suck!!! (was: should...

Well - you get the idea. If a newsreader has trouble recognizing that one thread
is part of another, what's the References header for then?

    
    

    
  
Re comment 23:
> That's quite a dumb rule, frankly spoken...

The _user_ can always change the subject, it is the user agent that AIUI is
forbidden to do this. 

BTW, since the user agent should have other ways than the subject to keep track
of threading, I think a change (or clearification) of the _standard_ would be a
good idea.

    
    

    
  
As the GNKSA maintainer, I just like to add my $.02.

In the past, predating any USEFOR/MESSFOR drafts and
no clear best practice to be distinguished, MacSOUP's
way of dealing with Subject header change/subthread
splits marked by `(was: ...)' has been judged contrary
to the letter of the GNKSA.

Even then a plausible argument could be made (but
wasn't; note that evaluations are the responsibility
of their authors, and only checked for basic sanity)
that this behaviour, while probably not in accordance
with the strict interpretation of the GNKSA wordings,
is in fact in total agreement with the GNKSA spirit.

Noting this, two things are to be remarked.  One is
that the GNKSA wording needs to be adapted to not
only make room for this best practice, but in fact
even encourage it.

The other is that, at least in my book, dropping the
`(was: ..)' upon replies is at least sane behaviour,
arguably recommendable even.


End of $.02.  Kudos for all the good work.

    
    

    
  
Since comment 25 make my objections invalid, I think this should be checked in ASAP.

    
    

    
  
Torben: the patch still needs super-review.

bienvenu: could you sr the patch?



(removing "needs a=" from status whiteboard. I do not intend to ask for approval
for this patch, even if it should get sr before the tree opens)
Whiteboard: fix in hand, needs sr=, needs a= → fix in hand, needs sr=

        Attachment #101762 -
        Flags: superreview?(bienvenu)

    
    

    
  
I'd be much happier if this patch was only for news. I realize this happens in
mailing lists as well, but that News Article Format document only applies to
newsgroups. But if you at least do some cursory checking of the references, I'll
allow it for mail too. So I think you should at least check the references
header to see if the subject had changed. Or even checking if the original
message was a reply to another message or not! In other words, if the original
msg hdr has no references, then you shouldn't strip the was: (right?) According
the document posted, you should at least try to check the references. If the
original msg hdr is gone, I'd be OK with assuming the subject changed, but not
checking at all seems counter to the recommendations of that document.

Here's how you would check, once you found a subject ending in "(was:"   - 

take the msg hdr, call GetNumReferences to see how many references it has. If it
has none, then you should not be stripping the "(was:". If it has one or more,
look at the first reference and call db->GetMsgHdrForMessageID(reference). If
you can't find a reference, we can assume it's missing and strip the "was:". But
if you find the first ref and it's subect contains ""(was:", I think you have to
assume that the original subject contained "(was:" and you should not be
stripping it out. Does this seem wrong or too hard?

    
    

    
  
David,

let me try to answer your suggestions.

> I'd be much happier if this patch was only for news. I realize this
> happens in mailing lists as well, but that News Article Format
> document only applies to newsgroups.

Formally correct, but as you say, mailing lists often follow usenet
guidelines. And it is not even possible to say if this is mail or news.
When you start composing a message, you don't know yet if the user
changes from one to another.

> But if you at least do some cursory checking of the references, I'll
> allow it for mail too. So I think you should at least check the
> references header to see if the subject had changed. Or even checking
> if the original message was a reply to another message or not!

This sound like near to impossible. Do we even have a chance to find
the previous message? What if it does not have References (some mail
readers don't use it)?

> In other words, if the original msg hdr has no references, then you
> shouldn't strip the was: (right?)

Why?

> According the document posted, you should at least try to check the
> references.

Where exactly is the requested?

> Here's how you would check, once you found a subject ending in "(was:"
> - take the msg hdr, call GetNumReferences to see how many references
> it has. If it has none, then you should not be stripping the "(was:".

As I said above, for mailing lists, it might fail, even though this
was an answer to an answer to an answer to ...

> If it has one or more, look at the first reference and call
> db->GetMsgHdrForMessageID(reference).

Does this work for mail and news at the same time? What happens if the
message is not in the database?

> If you can't find a reference, we can assume it's missing and strip
> the "was:". But if you find the first ref and it's subect contains
> ""(was:", I think you have to assume that the original subject
> contained "(was:" and you should not be stripping it out.

It is really hard to image a non-constructed situation where this
should not be removed. Even following your suggestion would only delay
the removal to the next message, which then is an answer.

> Does this seem wrong or too hard?

I think you are trying to be overly correct which might often fail in
the real world and bring no gain at some higher cost.

pi

    
    

    
  
>And it is not even possible to say if this is mail or news.
>When you start composing a message, you don't know yet if the user
>changes from one to another.

While this is true, I'd think that checking if it's news is fine if only done at
"reply-hitting" time.

However... um... bienvenu, aren't you contradicting yourself in your comment?
First you said I should check if it's news, but then you said I should check
references? Should I do both?



    
    

    
  
Not sure I see the contradiction - I said I'd be happier if it was just for
news, but I'd allow it for mail if you at least tried to check the references
for both. Where's the contradiction there? Mail has references too, right?

I don't understand why you say that checking the references is nearly
impossible, when it's actually trivial. It's also trivial to see if a compose
window is opened on a news message or a mail message, though I already said I
would accept it for mail.

If a message doesn't have any references, then it's not a reply. So, why would
you strip the (was: out of the subject? By definition, the composer of the
message would have to put that subject in there.  Or, are you saying there are
news/mail clients that do the was: thing, but add reference headers? That seems
implausible.

    
    

    
  
David,

> Mail has references too, right?

Not necessarily. Some readers set those, some don't. This is not something to
rely upon.

> I don't understand why you say that checking the references is nearly
> impossible, when it's actually trivial.

You have to make sure that you are able to find the message which might be
missing for various reasons. So relying on it makes the consequences somehow
arbitrary.

> It's also trivial to see if a compose
> window is opened on a news message or a mail message

Right, but it can be made a mail or a posting before sending. Also with
mail-to-news-gateways the distinction vanishes.

> If a message doesn't have any references, then it's not a reply.

Only for news.

> So, why would you strip the (was: out of the subject?

Because the references probably are missing by mistake.

> By definition, the composer of the
> message would have to put that subject in there.  Or, are you saying there are
> news/mail clients that do the was: thing, but add reference headers?

Absolutely.

Say you have a posting with references and
Subject: Re: old topic

A followup (of course still with references) has
Subject: Something completely different (was: old topic)

And the followup to that should also have references and
Subject: Re: Something completely different

pi

    
    

    
  
Comment on attachment 101762 [details] [diff] [review]
patch

un-requesting super-review per comments in bug

        Attachment #101762 -
        Flags: superreview?(bienvenu)

    
    

    
  
If you want to talk to the author of that draft document and see if he thinks we
should do it for both mail and news messages, and ignore reference headers, then
I'd go along.

    
    

    
  
Let me try to fully understand your proposal, David. You are saying you don't
like it for mail, but might allow it. As you say this feature is something which
is really used in mailing lists and mail2news gateways bring mail and news
closely togehter.

Then you suggest to do some kind of checking. Firstly, I think this is way too
unreliable to lead to reasonable results. Secondly, I don't really see it is
needed. Could you please give an example where (was: should not be removed and
the checking would help to avoid it?

pi

    
    

    
  
Adding GNKSA blocking based on comment 25.

David, could you please answer my last question (comment 35), so that we can
solve this issue? TIA

pi
Blocks: gnksa

    
    

    
  
I'd like to implement the draft. There's a reason the draft says that you can
verify if the (was: was added by looking at the reference headers; that's so we
don't remove a (was: that was part of an original subject.

For example:

To: friends
Subject: New email address: xxx@y.com (was: zzz@ddd.com - don't use old address!!!!

I'm cc'ing Seth to see what he thinks. If he thinks we should do this as is,
then it's fine with me.


    
    

    
  
Following your example. Say someone replies (no references, maybe in-reply-to).
Then it is our turn again. We cannot find if the (was: was there originally and
would strip it either way.

What should we do if the referenced article is not locally available? Fetch it
(for news)? If not it makes the decision somehow arbitrary.

pi

    
    

    
  
yes, by your argument, perhaps we shouldn't do this for mail. I would be fine
with that. I think we should err on the side of not doing this if we can't
verify that we're doing the right thing; you're arguing that we should always do
this and err on the side of crunching real subjects, because it would be rare.
It's a difference of opinion and I don't think either of us is going to convince
the other. Which was why I was hoping Seth could weigh in.

For news, we have all the headers locally so we just would look at the local
header. I know people make the argument that we should treat mail and news
exactly the same because mailing lists can get mirrored to newsgroups and vice
versa, but that ignores the differences between a centralized repository of
messages (a news server) and local mail stores. That's why the original ietf
draft can propose what it proposes - it's not worrying about the mail case at all.

    
    

    
  
I do not think that this should be marked as blocking the GNKSA tracking bug.
Until the GNKSA standard is updated according to Jeroen's comments, fixing this
bug could at best be said not to interfere with bug #12699.

That said, it sounds like a good idea, especially if the GNKSA is updated to
accommodate it.

    
    

    
  
Regarding references in mail:  mail clients that do not do usenet (i.e., most
mail clients) do not tend to support the References: header.  Some of them do
use an In-reply-to: header, but that gives only the direct parent, no
grandparents.  If you want to check references in mail, you have at least 
four cases to consider:
 1.  Message includes a References: header, and enough of the referenced
     messages can be fetched to make a determination one way or the other.
 2.  Message includes an In-Reply-To: header that, together with information
     in the parent post (considering all four possibilities again recursively)
     is enough to make a determination.
 3.  One or more of the ancestor messages is unavailable, preventing a
     determination from being made.
 4.  There are no References:, and tracing the In-Reply-To: header back
     (if it even exists) does not provide enough ancestors to make a
     determination, because the chain of parent articles is broken by a
     message that does not provide either header.

Sounds like work.  (If someone wants to do that work anyway, of course, I
won't argue against it...  I'm just saying it is more involved than looking
at a single header.)

    
    

    
  
bienvenu: OK, I now tried to implement your suggestion from comment 28. However,
I am not sure how I should get the message database - should I do it by doing
something like this (pseudo-code):
 folder = msgHdr->GetFolder;
 db = folder->GetMsgDatabase(nsnull);

or is there a better way?
the rest seems pretty straightforward to implement.

    
    

    
  
Christian, yes, you can get the folder from the msg hdr and the db from that
(though you need to check for a null folder or db - it shouldn't happen,  but in
some rare situations, it does)

    
    

    
  

      
      
      
      

        Attached patch
          
          
        Patch that implements comment 28 (obsolete)
        — 
          — Splinter Review
      

    


  
ok, this implements the suggestion from comment 28.

If you do not agree with this approach, please speak now.

        Attachment #101762 -
        Attachment is obsolete: true

        Attachment #110028 -
        Flags: review?(ducarroz)

    
    

    
  
Comment on attachment 110028 [details] [diff] [review]
Patch that implements comment 28

The patch looks good. But for the performance, it would be better I thing to
first check if the subject of the message contains "was" before checking with
the DB if it's ok to strip it!

    
    

    
  
Comment on attachment 110028 [details] [diff] [review]
Patch that implements comment 28

clearing review request, I'll attach a new patch in a second that implements
ducarroz's suggestion

        Attachment #110028 -
        Attachment is obsolete: true

        Attachment #110028 -
        Flags: review?(ducarroz)

        Attachment #110361 -
        Flags: review?(ducarroz)

    
    

    
  
Comment on attachment 110361 [details] [diff] [review]
patch v3

Better. R=ducarroz

        Attachment #110361 -
        Flags: review?(ducarroz) → review+

        Attachment #110361 -
        Flags: superreview?(bienvenu)

    
    

    
  
Comment on attachment 110361 [details] [diff] [review]
patch v3

sr=bienvenu, thx.

        Attachment #110361 -
        Flags: superreview?(bienvenu) → superreview+

    
    

    
  
checked in
Status: ASSIGNED → RESOLVED
Closed: 22 years ago
Resolution: --- → FIXED
Whiteboard: fix in hand, needs sr=
Target Milestone: mozilla1.2beta → mozilla1.3beta

    
    

    
  
Could you please describe exactly when it should be removed so I can verify this
bug.

pi

    
    

    
  
If it exists, it will not be removed if:
o It is the first message in a thread (ie. has no references)
o It is a follow-up to another message, but the subject of the first message 
already contains the was:

In all other cases, it will be removed, including if an error occurs during
above checks.


    
    

    
  
The second thing does not make much sense. So if someone forgets to remove it,
it will stay?

pi

    
    

    
  
No - if the message that started the thread had the was: alraedy, it will stay.

    
    

    
  
I see, you look at the first, not the previous message. That sounds better.

pi

    
    

    
  
Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.3b) Gecko/20030117, build 2003011712

It fails for me. news:3e37c05e$0$13932$3b214f66@news.univie.ac.at has the subject
"Neues Subject no reply ignore (was: pi's OjE-Test do not reply, really don't
reply, just ignore: הצ��)". All messages in the references have the previous
subject. When I hit reply, I get the full subject including the was part.

Could it be that the MIME encoding of the subject causes the problem?


Another failure with news:3e37c181$0$13932$3b214f66@news.univie.ac.at where the
subject changed compared to the previous message. The first in the thread indeed
has some (was:, but not the one changed here.

pi
Status: RESOLVED → REOPENED
Resolution: FIXED → ---

    
    

    
  
pi, firstly, I am unable to find these messages either on news.cis.dfn.de or via
groups.google.com, so I can't check about your first question.

as for the second message, that behaviour is how the code currently works. if
you don't like that, please file a new bug (not assigned to me, but please cc me).

    
    

    
  
hm, I could now get to that message. looks like the encoding is really the
problem, it looks like this:
Subject: =?ISO-8859-1?Q?Neues_Subject_no_reply_ignore_=28was=3A?=
 =?ISO-8859-1?Q?_pi=27s_OjE-Test_do_not_reply=2C_really_don?=
 =?ISO-8859-1?Q?=27t_reply=2C_just_ignore=3A_=E4=F6=FC=DF=29?=

retargeting
Priority: P2 → P4
Target Milestone: mozilla1.3beta → mozilla1.5alpha

    
    

    
  
As the person that did the GNKSA evaluation that failed MacSOUP for 
doing this, let me say that my biggest objection was that the choice was 
removed completely from the users control -- if there'd been a preference 
for turning this behavior on I'd have had no problem with it, if there'd been 
a preference for turning it /off/ I'd have accepted it.  But as long as it 
completely removes the users choice in the matter, I didn't feel I could 
sanction it as "good behavior" without a formal spec saying that it should 
be done.  Particularly given that there *are* newsreaders that do 
thread/grouping based upon the Subject, changing the Subject breaks 
that for those users.

In short, please include a way to turn this behavior on or off -- GUI or 
user.js, I don't care, but please include /some/ way for the user to have 
*his* say on the matter.

    
    

    
  
Actually, let's do this a bit differently.
I'll mark this bug fixed because it is.

pi, please file a new bug about that mime issue, ok?
planb: please also file a new bug.
please don't assign either of these bugs to me, but please either cc me on them
or mention them here. thanks.
Status: REOPENED → RESOLVED
Closed: 22 years ago → 22 years ago
Priority: P4 → P2
Resolution: --- → FIXED
Target Milestone: mozilla1.5alpha → mozilla1.3beta

    
    

    
  
As the GNKSA evaluator for MacSOUP, I have to say that the deciding 
factor on my failing it was that it didn't have an option to turn it on or off.  
Absent a standard saying that this is a required action, the user should 
have the ability to say whether or not it should be done (particularly given 
that some newsreaders do threading/grouping based upon the Subject, 
so that changing the Subject in this way breaks their grouping).

So, please include a way to turn it on or off.

    
  
Blocks: 191275

    
    

    
  
So be it.

See bug 191275.

pi
Status: RESOLVED → VERIFIED
Product: MailNews → Core
Product: Core → MailNews Core

          You need to log in
          before you can comment on or make changes to this bug.
        






  

    
  







  

    

      
Attachment

      

      
      
      
      
    

    

      

        

          

            
General

            

              Creator: 



            

            
Created: 

            
Updated: 

            
Size: