Closed
Bug 776438
Opened 12 years ago
Closed 10 years ago
Preference mailnews.reply_header_originalmessage should not be used for forwarded messages
Categories
(MailNews Core :: MIME, defect)
MailNews Core
MIME
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
|
iannbugzilla
:
review+
|
Details | Diff | Splinter Review |
1.66 KB,
patch
|
iannbugzilla
:
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).
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 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 4•11 years ago
|
||
(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)
Comment 6•11 years ago
|
||
I suppose, hardly a critical bug though ;)
Flags: needinfo?(mkmelin+mozilla)
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)
Should I fix the name of the MimeGetReplyHeaderOriginalMessage function and the corresponding variables to show it is about forwarding, not replying to?
Comment 10•10 years ago
|
||
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 11•10 years ago
|
||
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+
Assignee | ||
Comment 12•10 years ago
|
||
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.
Assignee | ||
Comment 13•10 years ago
|
||
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)
Comment 14•10 years ago
|
||
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.
Assignee | ||
Comment 15•10 years ago
|
||
Sure, thanks.
Attachment #8362719 -
Flags: review?(iann_bugzilla)
Attachment #8362719 -
Flags: review?(iann_bugzilla) → review+
Keywords: checkin-needed
Comment 16•10 years ago
|
||
https://hg.mozilla.org/comm-central/rev/996fcc9d6d30 https://hg.mozilla.org/comm-central/rev/67785d81fb61
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 29.0
Reporter | ||
Comment 17•10 years ago
|
||
Thanks!
You need to log in
before you can comment on or make changes to this bug.
Description
•