Closed Bug 776438 Opened 8 years ago Closed 6 years ago

Preference mailnews.reply_header_originalmessage should not be used for forwarded messages

Categories

(MailNews Core :: MIME, defect, minor)

defect
Not set
minor

Tracking

(Not tracked)

RESOLVED FIXED
Thunderbird 29.0

People

(Reporter: mmitar, Assigned: aceman)

References

Details

(Keywords: regression, Whiteboard: [thunderbird-13-unaffected][thunderbird-14-affected])

Attachments

(2 files, 1 obsolete file)

7.81 KB, patch
iann_bugzilla
: review+
Details | Diff | Splinter Review
1.66 KB, patch
iann_bugzilla
: review+
Details | Diff | Splinter Review
User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.7; rv:14.0) Gecko/20100101 Firefox/14.0.1
Build ID: 20120713134347

Steps to reproduce:

After bug #621264 was "fixed", mailnews.reply_header_originalmessage preference is used also for header of forwarded messages. This is in my opinion (and workflow) wrong, there should be mailnews.forward_header_originalmessage preference used instead (not yet existent).
Blocks: 621264
So the request here is to decouple the "original message" header of forwarded messages from replied messages, by creating a separate pref for it.

Yes, the request in bug 621264 was to use the mailnews.reply_header_originalmessage pref but it may be because there was no other way to set the forwarded header. If we create a new pref then the use case in bug 621264 will be covered too, only that the user must set both prefs now.

I think I can do this if it is considered wanted.
Assignee: nobody → acelists
Severity: normal → enhancement
Status: UNCONFIRMED → NEW
Component: Preferences → MIME
Ever confirmed: true
OS: Mac OS X → All
Product: Thunderbird → MailNews Core
Hardware: x86 → All
I just voted for it. :-)
Version: 14 → Trunk
I meant the TB developers ;)

Ian, this could be considered a regression starting at TB14 so the version field was not that bad at having value 14. I wonder if I should mark it that way or leave it at enhancement. The previous behaviour has changed, for some users into being wrong now.
(In reply to :aceman from comment #3)
> I meant the TB developers ;)
> 
> Ian, this could be considered a regression starting at TB14 so the version
> field was not that bad at having value 14. I wonder if I should mark it that
> way or leave it at enhancement. The previous behaviour has changed, for some
> users into being wrong now.

Comment #3 is already more than a year old but I suppose it has some merit.

David, do you think this bug sould be WONTFIX or that aceman should take it (if he's still willing)?
Aceman, are you still just waiting for a go-ahead signal or should this bug be reassigned to nobody@ ?
Severity: enhancement → minor
Flags: needinfo?(acelists)
Keywords: regression
Whiteboard: [thunderbird-13-unaffected][thunderbird-14-affected]
Yes, I am waiting for the go-ahead signal :)
I doubt David Bienvenu will respond on this case. Let's ask some of the current stakeholders.
Flags: needinfo?(mkmelin+mozilla)
Flags: needinfo?(iann_bugzilla)
Flags: needinfo?(acelists)
Flags: needinfo?(Pidgeot18)
I suppose, hardly a critical bug though ;)
Flags: needinfo?(mkmelin+mozilla)
I have no disagreement here.
Flags: needinfo?(Pidgeot18)
Attached patch patch (obsolete) — Splinter Review
Would this work?
I never understand why the reply and forward quoting code is so different (one in mimedrft.cpp, one in nsMsgCompose.cpp).
Attachment #8339547 - Flags: review?(mkmelin+mozilla)
Attachment #8339547 - Flags: review?(iann_bugzilla)
Attachment #8339547 - Flags: review?(Pidgeot18)
Status: NEW → ASSIGNED
Should I fix the name of the MimeGetReplyHeaderOriginalMessage function and the corresponding variables to show it is about forwarding, not replying to?
Comment on attachment 8339547 [details] [diff] [review]
patch

Review of attachment 8339547 [details] [diff] [review]:
-----------------------------------------------------------------

Works for me. I wouldn't bother renaming.
Attachment #8339547 - Flags: review?(mkmelin+mozilla) → review+
Comment on attachment 8339547 [details] [diff] [review]
patch

Review of attachment 8339547 [details] [diff] [review]:
-----------------------------------------------------------------

I disagree, I'd rather see the function renamed if it's not too much trouble.
Attachment #8339547 - Flags: review?(Pidgeot18) → review+
No problem, it is an internal helper function added by me in bug 621264, because I named it after the pref name (reply_*) fogetting we actually use it to create a header for forwarded messages. Which now will be clear also from the new pref name. There should be no callers outside mimedrft.cpp.
Attached patch patch v2Splinter Review
OK, done.
Attachment #8339547 - Attachment is obsolete: true
Attachment #8339547 - Flags: review?(iann_bugzilla)
Attachment #8347468 - Flags: review?(iann_bugzilla)
Attachment #8347468 - Flags: review?(iann_bugzilla) → review+
Flags: needinfo?(iann_bugzilla)
It would be good to add a services.sync.pref.sync for this new pref similar to http://mxr.mozilla.org/comm-central/source/suite/browser/browser-prefs.js#896 probably just after line 872 (alphabetically) but that could be spun off into a separate SM bug if you prefer.
Sure, thanks.
Attachment #8362719 - Flags: review?(iann_bugzilla)
Attachment #8362719 - Flags: review?(iann_bugzilla) → review+
Keywords: checkin-needed
https://hg.mozilla.org/comm-central/rev/996fcc9d6d30
https://hg.mozilla.org/comm-central/rev/67785d81fb61
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 29.0
Thanks!
You need to log in before you can comment on or make changes to this bug.