Closed
Bug 399446
Opened 17 years ago
Closed 11 years ago
Newsgroup: should not be displayed if there is no newsgroup account
Categories
(Thunderbird :: Message Compose Window, defect)
Thunderbird
Message Compose Window
Tracking
(Not tracked)
RESOLVED
FIXED
Thunderbird 24.0
People
(Reporter: hidenosuke, Assigned: aceman)
References
Details
(Keywords: polish)
Attachments
(1 file, 3 obsolete files)
7.48 KB,
patch
|
aceman
:
review+
|
Details | Diff | Splinter Review |
Steps to reproduce: 1. Make sure you don't have newsgroup account. 2. Open Compose Window. 3. Click combo box displayed "To:". Exptected result: No Newsgroup: in the drop down list. Actual result: There is Newsgroup: in the drop down list. Original bug report written in Japanese is http://bugzilla.mozilla.gr.jp/show_bug.cgi?id=4130
Reporter | ||
Comment 1•17 years ago
|
||
I observed this bug with 2007101104-trunk/Linux.
Updated•17 years ago
|
OS: Linux → All
Hardware: PC → All
Comment 2•12 years ago
|
||
assign to me, please
Updated•12 years ago
|
Assignee: nobody → blog
Updated•12 years ago
|
Status: NEW → ASSIGNED
Jens, are you working on this? Also, should this enable "Newsgroup" option if there exists any NNTP account in the profile? Or only if a NNTP account is chosen in the From field?
Version: unspecified → Trunk
Comment 4•11 years ago
|
||
(In reply to :aceman from comment #3) > Jens, are you working on this? Sorry, not yet. First wanna wrap up bug 99019 properly ... > Also, should this enable "Newsgroup" option if there exists any NNTP account > in the profile? Or only if a NNTP account is chosen in the From field? Hu, this is a tough question. I'd prefer the latter ...
I mean if you are not attached to fix this specific bug personally, I could take it too.
Comment 6•11 years ago
|
||
Yes, please do. Once I manage to get the infrastructure for my first case working, I'll hopefully be faster with the following ones ...
This seems to do it. For mkmelin: This intentionally changes "accountname" to "accountkey" in the statement: identityElement.selectedItem.getAttribute("accountkey"); identityElement.setAttribute("accountkey", accountKey); It seems "accountname" was a mistake as the attribute on the menuitems is accountkey. It was not detected that we were getting always "" as there does not seem to be any user of this functionality. But I think we can keep it as it may be useful. The question is whether to also change the attribute name on the patent menulist.
Attachment #753804 -
Flags: ui-review?(bwinton)
Attachment #753804 -
Flags: review?(mkmelin+mozilla)
Attachment #753804 -
Flags: feedback?(blog)
Comment 9•11 years ago
|
||
Comment on attachment 753804 [details] [diff] [review] patch Review of attachment 753804 [details] [diff] [review]: ----------------------------------------------------------------- Looks fine to me ... I think I already looked at that file before, but I find it quite hard to understand due to the lack of comments ... ::: mail/components/compose/content/MsgComposeCommands.js @@ +4146,5 @@ > + let addrWidget = document.getElementById("addressingWidget"); > + for (let row = 0; row < addrWidget.itemCount; row++) { > + let addrRow = addrWidget.getItemAtIndex(row); > + if (!addrRow.hasAttribute("_isDummyRow")) { > + let addrType = addrRow.firstChild.firstChild; For me, it would help to explain what each ".firstChild" is.
Comment 10•11 years ago
|
||
Comment on attachment 753804 [details] [diff] [review] patch Review of attachment 753804 [details] [diff] [review]: ----------------------------------------------------------------- Some thoughts: - we should probably do this for mail too, for people who only use tb as a newsreader - Followup-To is news specific, and i think we can hide it if there are no news accounts set up. You can set it in mails too, and it will be shown but have to be manually clicked for replies to go there. I suppose that's another bug. Anyway i doubt it's a common use case. - probably good to put the functionality in a separate function like hideIrrelvantAddressingOptions. - not sure it should be in loadidentity or in onopopushowing ::: mail/components/compose/content/MsgComposeCommands.js @@ +4149,5 @@ > + if (!addrRow.hasAttribute("_isDummyRow")) { > + let addrType = addrRow.firstChild.firstChild; > + if (addrType.id.startsWith("addressCol1#")) > + addrType.querySelector('[value="addr_newsgroups"]') > + .collapsed = hideNews; can't you just querySelector this from querySelector?
Attachment #753804 -
Flags: review?(mkmelin+mozilla)
Assignee | ||
Comment 11•11 years ago
|
||
(In reply to Magnus Melin from comment #10) > Comment on attachment 753804 [details] [diff] [review] > patch > > Review of attachment 753804 [details] [diff] [review]: > ----------------------------------------------------------------- > > Some thoughts: > - we should probably do this for mail too, for people who only use tb as a > newsreader I test if this is even possible yet. So you mean to hide 'To', 'CC' and 'BCC' ? But in the Account manager it is possible to set predefined CC and BCC on a News account too (the Copies&Folders pane). Or do you propose only to hide 'To'? > - Followup-To is news specific, and i think we can hide it if there are no > news accounts set up. You can set it in mails too, and it will be shown but > have to be manually clicked for replies to go there. > I suppose that's another bug. So I didn't understand this. Is it supposed to do anything on non-News on not? > Anyway i doubt it's a common use case. > - probably good to put the functionality in a separate function like > hideIrrelvantAddressingOptions. Will do. > - not sure it should be in loadidentity or in onopopushowing LoadIdentity will probably be used about once per window. The address type popup may be used several times (and identity has not changed). So if no other difference, the LoadIdentity way may be more efficient. > ::: mail/components/compose/content/MsgComposeCommands.js > @@ +4149,5 @@ > > + if (!addrRow.hasAttribute("_isDummyRow")) { > > + let addrType = addrRow.firstChild.firstChild; > > + if (addrType.id.startsWith("addressCol1#")) > > + addrType.querySelector('[value="addr_newsgroups"]') > > + .collapsed = hideNews; > > can't you just querySelector this from querySelector? ???
Comment 12•11 years ago
|
||
(In reply to :aceman from comment #11) > (In reply to Magnus Melin from comment #10) > > Comment on attachment 753804 [details] [diff] [review] > > patch > > > > Review of attachment 753804 [details] [diff] [review]: > > ----------------------------------------------------------------- > > > > Some thoughts: > > - we should probably do this for mail too, for people who only use tb as a > > newsreader > I test if this is even possible yet. So you mean to hide 'To', 'CC' and > 'BCC' ? Yeah, or strictly speaking you may need to only have an smtp set, didn't check how that works in practice though. > But in the Account manager it is possible to set predefined CC and BCC on a > News account too (the Copies&Folders pane). Or do you propose only to hide > 'To'? Arguably the auto options should be disabled unless an email account is set, imho. > > - Followup-To is news specific, and i think we can hide it if there are no > > news accounts set up. You can set it in mails too, and it will be shown but > > have to be manually clicked for replies to go there. > > I suppose that's another bug. > So I didn't understand this. Is it supposed to do anything on non-News on > not? I don't know if it has a defined behavior... > > can't you just querySelector this from querySelector? > > ??? Sorry, i meant can't you directly from addrWidget?
Assignee | ||
Comment 13•11 years ago
|
||
OK, I have tested it and when sending to a Newsgroup I could add a To and Bcc addressee (myself) and it worked (message delivered). So I think I can not disable those (in compose and in account manager). The Followup-to is still open. Can anybody explain what it does and if it can be set when sending from a non-News account?
Flags: needinfo?(Pidgeot18)
Comment 14•11 years ago
|
||
(In reply to :aceman from comment #13) > The Followup-to is still open. Can anybody explain what it does and if it > can be set when sending from a non-News account? https://tools.ietf.org/html/rfc5536#section-3.2.6 defines it for Netnews articles. https://tools.ietf.org/html/rfc5322, in contrast, does not define such a header.
Comment 15•11 years ago
|
||
(In reply to :aceman from comment #13) > OK, I have tested it and when sending to a Newsgroup I could add a To and > Bcc addressee (myself) and it worked (message delivered). So I think I can > not disable those (in compose and in account manager). Huh? Without any mail account set up, how would that be possible?
Assignee | ||
Comment 16•11 years ago
|
||
(In reply to Magnus Melin from comment #15) > Huh? Without any mail account set up, how would that be possible? I had mail accounts set up. But I specifically composed from the Newsgroup account (selecting a newspost and clicking Reply). So I don't think it should have touched any of the mail accounts.
Comment 17•11 years ago
|
||
If you have a mail account it will work (probably uses the default), but without mail set up it can't work.
Assignee | ||
Comment 18•11 years ago
|
||
I did test it, empty profile, only NNTP account set up, add SMTP server. Reply to a news post, change Newsgroup in the dropdown to To, write some email address. Message arrived just fine.
Comment 19•11 years ago
|
||
(In reply to :aceman from comment #13) > The Followup-to is still open. Can anybody explain what it does and if it > can be set when sending from a non-News account? Followup-To is to Newsgroups as Reply-To is to To. Nothing gets sent to it; it's merely an indicator to clients to how to treat reply.
Flags: needinfo?(Pidgeot18)
Assignee | ||
Comment 20•11 years ago
|
||
(In reply to Joshua Cranmer [:jcranmer] from comment #19) > Followup-To is to Newsgroups as Reply-To is to To. Nothing gets sent to it; > it's merely an indicator to clients to how to treat reply. So it not needed when posting from a non-News account? Can we hide it?
Flags: needinfo?(Pidgeot18)
Comment 21•11 years ago
|
||
(In reply to :aceman from comment #20) > (In reply to Joshua Cranmer [:jcranmer] from comment #19) > > Followup-To is to Newsgroups as Reply-To is to To. Nothing gets sent to it; > > it's merely an indicator to clients to how to treat reply. > > So it not needed when posting from a non-News account? Can we hide it? It only makes sense if you add a Newsgroup header.
Updated•11 years ago
|
Flags: needinfo?(Pidgeot18)
Assignee | ||
Comment 22•11 years ago
|
||
Attachment #753804 -
Attachment is obsolete: true
Attachment #753804 -
Flags: ui-review?(bwinton)
Attachment #753804 -
Flags: feedback?(blog)
Attachment #757569 -
Flags: ui-review?(bwinton)
Attachment #757569 -
Flags: review?(mkmelin+mozilla)
Comment 23•11 years ago
|
||
Comment on attachment 757569 [details] [diff] [review] patch v2 It would be cool if this updated the list when I opened the compose window, and then created a new newsgroup account, but I'm pretty happy with it the way it is if you didn't feel like changing that. :) ui-r=me.
Attachment #757569 -
Flags: ui-review?(bwinton) → ui-review+
Assignee | ||
Comment 24•11 years ago
|
||
It should update the list of addressee types if you select the newly created account in the From dropdown. If you say the account is not yet available in the dropdown, then that is a different bug :)
Comment 25•11 years ago
|
||
Comment on attachment 757569 [details] [diff] [review] patch v2 Review of attachment 757569 [details] [diff] [review]: ----------------------------------------------------------------- I get a busted addressing area (missing To:/Cc: dropdowns) if i start by selecting a newsgroup and clicking write, then try to change to a mail account. Didn't look into why, but nothing in the error console. Also, would be nice with a mozmill test for this.
Attachment #757569 -
Flags: review?(mkmelin+mozilla) → review-
Assignee | ||
Comment 26•11 years ago
|
||
In my previous tests even when the "Newsgroup" item was selected, collapsing it in the dropdown did not affect the appearance of the menu ("Newsgroup" was still shown even if no longer selectable). In your scenario, collapsing the item collapses the whole menu. If I find no better sollution, maybe changing collapse to disabled would work.
Assignee | ||
Comment 27•11 years ago
|
||
Sorry, wrong. The switch to querySelectorAll caused it to return also the parent menulist together with the menuitems so it got collapsed explicitly too. I fixed that and collapse can be used. Working on the test now.
Assignee | ||
Comment 28•11 years ago
|
||
The fix and test.
Attachment #757569 -
Attachment is obsolete: true
Attachment #760117 -
Flags: review?(mkmelin+mozilla)
Comment 29•11 years ago
|
||
Comment on attachment 760117 [details] [diff] [review] patch v3 Review of attachment 760117 [details] [diff] [review]: ----------------------------------------------------------------- Nice! r=mkmelin ::: mail/components/compose/content/MsgComposeCommands.js @@ +4140,5 @@ > + .querySelectorAll('menuitem[value="addr_newsgroups"], menuitem[value="addr_followup"]'); > + // Collapsing the menuitem only prevents it getting chosen, it does not > + // affect the menulist widget display when Newsgroup is already selected. > + for (let item of newsTypes) > + item.collapsed = hideNews; please always use braces for for loops
Attachment #760117 -
Flags: review?(mkmelin+mozilla) → review+
Assignee | ||
Comment 30•11 years ago
|
||
Thanks.
Attachment #760117 -
Attachment is obsolete: true
Attachment #760140 -
Flags: review+
Flags: in-testsuite+
Keywords: checkin-needed
Comment 31•11 years ago
|
||
https://hg.mozilla.org/comm-central/rev/16e20df57d08
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 24.0
Comment 32•11 years ago
|
||
I loaded Thunderbird 24.0 and can no longer send messages to my internal newsgroups (as the to newsgroup option has disappeared). Please restore this function
Assignee | ||
Comment 33•11 years ago
|
||
What are internal newsgroups? What type of account do you use to send to them? Can you tell the exacts steps you try to do?
Comment 34•11 years ago
|
||
We have newsgroups that are behind our firewall at news.internaldomain.com they are on an AIX unix machine running an inn newsgroup server. We connect to the server vi NNTP at port 119, until patch 24 we could cc the newsgroup because "newsgroup" was in the "TO" dropdown when we composed or replied to an email. The email client is IMAP to port 142 of the same Unix server. Let me know if you need more details, Thanks, Steve
Comment 35•11 years ago
|
||
It's visible if you're writing from a newsgroup account (and you can temporarily switch to one). Granted, i agree this makes mail+news postings very difficult. Would be better if it hid depending on the presence of a newsgroup account.
Assignee | ||
Comment 36•11 years ago
|
||
Magnus, if you have selected a mail account as the From and still post to both mail & news addresses, is that supposed to work? But it will send using your mail account that may no have the proper settings.
Comment 37•11 years ago
|
||
That's supposed to work yes. It will send to the mail addresses using mail protocols, and do a post to the newsgroup using nntp.
Assignee | ||
Comment 38•11 years ago
|
||
And what account will it use for the NNTP protocol? Isn't sending to news just SMTP?
Comment 39•11 years ago
|
||
AFAIK, sending to news is just using NNTP (no smtp involved).
Assignee | ||
Comment 40•11 years ago
|
||
I don't think so. There is SMTP server setting in the account manager when you view a news account. I think sending to News is through normal SMTP just downloading is over NNTP protocol.
Comment 41•11 years ago
|
||
(In reply to :aceman from comment #40) > I don't think so. There is SMTP server setting in the account manager when > you view a news account. I think sending to News is through normal SMTP just > downloading is over NNTP protocol. jcranmer?
Comment 42•11 years ago
|
||
I'd expect that to be the smtp used when sending from a news identity, and e.g. ccing mail people (just in case you have multiple mail accounts set up you get to choose)
Comment 43•11 years ago
|
||
(In reply to :aceman from comment #40) > I don't think so. There is SMTP server setting in the account manager when > you view a news account. I think sending to News is through normal SMTP just > downloading is over NNTP protocol. And you would be wrong. The SMTP setting is to support sending to both newsgroups and other people, such as when I cross-post between m.d.a.t (a newsgroup) and tb-planning (a mailing list). (In reply to :aceman from comment #38) > And what account will it use for the NNTP protocol? Isn't sending to news > just SMTP? If you are using a non-news identity, I believe it picks the news server that has that newsgroup; if none of them have it, I believe it defaults to the "first" one. There are some bugs somewhere about having an explicit default news account for this sort of stuff (and no-authority news URIs).
Assignee | ||
Comment 44•11 years ago
|
||
Ok, thanks. So the answer is we need a News account in TB for posting to a newsgroup?
Assignee | ||
Comment 45•11 years ago
|
||
Steve, can you confirm you have a News account configured in TB?
Comment 46•11 years ago
|
||
(In reply to :aceman from comment #45) > Steve, can you confirm you have a News account configured in TB? Yes, we have 2 accounts set up, a standard imap email account and a news group account (pointing to port 119 on the same server as the imap email). We use the news group as a common customer specific repository, so we often forward to or cc the newsgroup when sending emails from the imap account.
Assignee | ||
Comment 47•11 years ago
|
||
OK, I have split the problem into bug 922614 (to ease the tracking into which branches the fix must go) and will work on it.
You need to log in
before you can comment on or make changes to this bug.
Description
•