Closed
Bug 1008822
Opened 10 years ago
Closed 10 years ago
Forwarding RSS post as inline does not use default account identity
Categories
(Thunderbird :: Message Compose Window, defect)
Tracking
(Not tracked)
RESOLVED
FIXED
Thunderbird 33.0
People
(Reporter: jwbodnar, Assigned: alta88)
References
Details
(Keywords: regression)
Attachments
(1 file, 3 obsolete files)
3.24 KB,
patch
|
mkmelin
:
review+
|
Details | Diff | Splinter Review |
User Agent: Mozilla/5.0 (Windows NT 5.1; rv:29.0) Gecko/20100101 Firefox/29.0 (Beta/Release) Build ID: 20140506152807 Steps to reproduce: Selected a message in an RSS Feed: 1. Click the "Forward" button in the reading pane or the "Forward" button in the toolbar or specifically the "Forward Inline" toolbar button option. Actual results: From address of the message compose window defaults to first e-mail identity name in alphabetical order. Expected results: From address of the message compose window should default to default account (e-mail address). Note, this IS what happens if "Forward As Attachment" is selected from the toolbar button.
identity fix, plus web page forward link fix (considers both tb and sm).
Assignee: nobody → alta88
Status: UNCONFIRMED → NEW
Ever confirmed: true
Attachment #8421871 -
Flags: review?(mkmelin+mozilla)
Comment 3•10 years ago
|
||
Comment on attachment 8421871 [details] [diff] [review] identity.patch Review of attachment 8421871 [details] [diff] [review]: ----------------------------------------------------------------- Sorry for the delay ::: mailnews/extensions/newsblog/content/newsblogOverlay.js @@ +42,5 @@ > aType, aFormat, aIdentity, aMsgWindow) > { > + // Do not use the header derived identity in ComposeMessage() for feeds; it may > + // not be the default. Compose correctly gets the default if identity is null. > + aIdentity = null; I don't think this is right, if the function has a parameter it should be used. So, either fix the callers to pass null, or remove the param from the function signature. (I didn't verify the bug exist, but I guess it does if allIdentities order doesn't start with the default account, if so we get the wrong identity in some other edge cases also.)
Attachment #8421871 -
Flags: review?(mkmelin+mozilla) → review-
Attachment #8421871 -
Attachment is obsolete: true
Attachment #8428670 -
Flags: review?(mkmelin+mozilla)
Keywords: regression
Attachment #8428670 -
Flags: review?(mkmelin+mozilla)
something basic has broken with getting the default identity.
ok, so it appears that for non identity accounts, setting the identity explicitly from the default account and its default identity is required, so that what the user set in the ui relates to what actually happens on compose. attempting to get an identity from headers for feeds is a regression from bug 714570, but it's likely all the guesswork at an identity in mailCommands.js didn't work right in a multi account/multi identity scenario anyway.
Attachment #8428670 -
Attachment is obsolete: true
Attachment #8428753 -
Flags: review?(mkmelin+mozilla)
Comment 7•10 years ago
|
||
Comment on attachment 8428753 [details] [diff] [review] identity.patch Review of attachment 8428753 [details] [diff] [review]: ----------------------------------------------------------------- ::: mailnews/extensions/newsblog/content/newsblogOverlay.js @@ +46,5 @@ > + // If there is no server identity (currently true for feed accounts but > + // not necessarily true for feed messages in non feed account folders), > + // use the defaults. > + aIdentity = MailServices.accounts.defaultAccount.defaultIdentity; > + } this block is not necessary, as null will imply the default account, default identity @@ +53,5 @@ > + let tabmail = document.getElementById("tabmail"); > + let is3pane = tabmail && tabmail.selectedTab && tabmail.selectedTab.mode ? > + tabmail.selectedTab.mode.type == "folder" : false; > + let showingwebpage = ("FeedMessageHandler" in window) && !is3pane && > + FeedMessageHandler.onOpenPref == FeedMessageHandler.kOpenWebPage; why wouldn't I be showing webpage in the 3pane?
(In reply to Magnus Melin from comment #7) > Comment on attachment 8428753 [details] [diff] [review] > identity.patch > > Review of attachment 8428753 [details] [diff] [review]: > ----------------------------------------------------------------- > > ::: mailnews/extensions/newsblog/content/newsblogOverlay.js > @@ +46,5 @@ > > + // If there is no server identity (currently true for feed accounts but > > + // not necessarily true for feed messages in non feed account folders), > > + // use the defaults. > > + aIdentity = MailServices.accounts.defaultAccount.defaultIdentity; > > + } > > this block is not necessary, as null will imply the default account, default > identity > true. > @@ +53,5 @@ > > + let tabmail = document.getElementById("tabmail"); > > + let is3pane = tabmail && tabmail.selectedTab && tabmail.selectedTab.mode ? > > + tabmail.selectedTab.mode.type == "folder" : false; > > + let showingwebpage = ("FeedMessageHandler" in window) && !is3pane && > > + FeedMessageHandler.onOpenPref == FeedMessageHandler.kOpenWebPage; > > why wouldn't I be showing webpage in the 3pane? if you are, it's governed correctly by gShowFeedSummary; the var really means showinginwebpage only for a tab or window, for which gShowFeedSummary isn't necessarily correct and onOpenPref is.
Attachment #8428753 -
Attachment is obsolete: true
Attachment #8428753 -
Flags: review?(mkmelin+mozilla)
Attachment #8432272 -
Flags: review?(mkmelin+mozilla)
Comment 10•10 years ago
|
||
(In reply to Magnus Melin from comment #3) > (I didn't verify the bug exist, but I guess it does if allIdentities order doesn't start with the default account, > if so we get the wrong identity in some other edge cases also.) From: is "From: <Seunguck Lee>", "From: MySQL Bugs", like one, no To:/CC:/Delivered-To:, no X-Account-Key:, no X-Identity-Key,.... if RSS Feed => Mail sent to me by BCC:, with wrong mail address or Mailing list. "First identity of first account in mail.accountmanager.accounts" was used for Forward of RSS Feed in Tb 24.5.0. This looks final fall back identity("default account doesn't have identity" case, "default account points non-existent account" case, etc.).
Comment 11•10 years ago
|
||
(In reply to alta88 from comment #8) > > why wouldn't I be showing webpage in the 3pane? > > if you are, it's governed correctly by gShowFeedSummary; the var really > means showinginwebpage only for a tab or window, for which gShowFeedSummary > isn't necessarily correct and onOpenPref is. We really should clean up that mess...
Updated•10 years ago
|
Attachment #8432272 -
Flags: review?(mkmelin+mozilla) → review+
Keywords: checkin-needed
Comment 12•10 years ago
|
||
https://hg.mozilla.org/comm-central/rev/935a121a00df
Status: NEW → RESOLVED
Closed: 10 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 33.0
Updated•10 years ago
|
Component: Untriaged → Message Compose Window
You need to log in
before you can comment on or make changes to this bug.
Description
•