1792362 - "Get Messages" dropdown > News server name doesn't fetch new messages

Status: RESOLVED FIXED

(MailNews Core :: Networking: NNTP, defect)

Description

[b2]

User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/105.0.0.0 Safari/537.36

Steps to reproduce:

On a server that is configured not to fetch messages automatically, fetching messages manually via the "Get Messages" button doesn't work in TB 102.

Clicking onto the server in the folder tree and selecting "Get Messages" from the context menu still works.

The two menu items do different things: MsgGetMessagesForAccount(event.target._folder) and MsgGetMessage() respectively.

Comment 1

[b2]

The problems is more complicated: Selecting the news server from "Get Messages" button dropdown does retrieve messages when the root folder is selected in the folder tree, it doesn't download when a newsgroup is selected. In case where the download happens (server selected in folder tree), the newsgroup which receives new messages doesn't get the yellow biff symbol, so it appears that nothing was downloaded.

Comment 2

[Ping Chen [:rnons] (away)]

Attachment: Bug 1792362 - Fix fetching nntp messages from Get Messages dropdown. r=mkmelin,benc

Comment 3

[Ping Chen [:rnons] (away)]

Seems to be a quite old problem.

Comment 4

[b2]

(In reply to Ping Chen (:rnons) from comment #3)

Seems to be a quite old problem.

Indeed, thanks for fixing it!

Comment 5

[b2]

Out of interest, why was this block moved up?

  nsCOMPtr<nsINntpIncomingServer> nntpServer;
  rv = GetNntpServer(getter_AddRefs(nntpServer));
  NS_ENSURE_SUCCESS(rv, rv);

Comment 6

[Ping Chen [:rnons] (away)]

(In reply to b2 from comment #5)

Out of interest, why was this block moved up?

  nsCOMPtr<nsINntpIncomingServer> nntpServer;
  rv = GetNntpServer(getter_AddRefs(nntpServer));
  NS_ENSURE_SUCCESS(rv, rv);

Reverted, I was trying to get nsIMsgIncomingServer from nntpServer at first, later found GetServer can be used.

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/30cbbf5e4bdd
Fix fetching nntp messages from Get Messages dropdown. r=mkmelin,benc

Comment 8

[b2]

Sorry about revisiting this. There are four UI elements to get news messages:

First:
Context menu: MsgGetMessage(). That gets messages for the selected newsgroup or the all newsgroups on the selected server.

Second:
Top level button "Get Messages": MsgGetMessagesForAccount(). With no folder, that maps to MsgGetMessage() via
https://searchfox.org/comm-central/rev/e6fd9cc854a8043e277b6846cf28d8f6f7a159c0/mail/base/content/mailWindowOverlay.js#1889
https://searchfox.org/comm-central/rev/e6fd9cc854a8043e277b6846cf28d8f6f7a159c0/mail/base/content/mail3PaneWindowCommands.js#695

Third:
First menu entry on drop down: Get All New Messages (cmd_getMsgsForAuthAccounts): Appears to do nothing for news, regardless of folder tree selection. Note: When a NG is visited later, that NG is retrieved, so this adds to the confusion.

Fourth:
Server entries on the drop down: MsgGetMessagesForAccount(event.target._folder): That's the function that was fixed. Now gets all newsgroups on the selected server. However, it also gets all newsgroups on the server, even if a newsgroup is selected in the folder tree. Is this intended? Looking at the code
https://searchfox.org/comm-central/rev/e6fd9cc854a8043e277b6846cf28d8f6f7a159c0/mailnews/news/src/nsNewsFolder.cpp#799-814
it's meant just to get the selected newsgroup. (That's with the patch applied to a 102 ESR build).

Reopen this bug or new bug? Sorry about the deficient initial bug report.

Comment 9

[b2]

Debugged "fourth" a bit: Regardless of newsgroup or server selection in the tree, isNewsServer is always returned true here:
https://searchfox.org/comm-central/rev/e6fd9cc854a8043e277b6846cf28d8f6f7a159c0/mailnews/news/src/nsNewsFolder.cpp#788

Note that comment #2 was wrong. We got confused by the newsgroups fetching new messages when their folder is opened. Hence reverting the summary.

Summarising: "First" and "Fourth" are likely OK. "Second" is a bit surprising since despite the tooltip, if a newsgroup is selected, only the newsgroup is fetched, not all newsgroups on the server. "Third" is broken.

Comment 10

[Ping Chen [:rnons] (away)]

it also gets all newsgroups on the server, even if a newsgroup is selected in the folder tree. Is this intended?

Yes, it's intended, since the menu item is the server name, I interpret it as fetching all subscribed groups on that server.

I don't know if we want to change the behavior of the 2nd, please open a new bug for it.

I will make a new patch here to fix the 3rd.

Comment 11

[Ping Chen [:rnons] (away)]

Attachment: Bug 1792362 - Get All New Messages button should work for nntp servers. r=mkmelin

Comment 12

[b2]

Thank you for the comment and the fix.

(In reply to Ping Chen (:rnons) from comment #10)

I don't know if we want to change the behavior of the 2nd, please open a new bug for it.
I will make a new patch here to fix the 3rd.

It's debatable what a button click on "Get Messages" should do. The label matches "Get Messages" on the context menu, so maybe the behaviour should be the same. It's only unfortunate that the tooltip isn't clear.

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/fb2dec42c8e0
Get All New Messages button should work for nntp servers. r=mkmelin

Comment 14

[b5]

Please backport to ESR 102 (like bug 1794185).

Comment 15

[Ping Chen [:rnons] (away)]

Comment on attachment 9297067 [details]
Bug 1792362 - Get All New Messages button should work for nntp servers. r=mkmelin

Request for the two patches.
[Approval Request Comment]
Regression caused by (bug #): old bug
User impact if declined: "Get Messages" dropdown doesn't work for nntp servers.
Testing completed (on c-c, etc.): beta
Risk to taking this patch (and alternatives if risky): low

Comment 16

[Wayne Mery (:wsmwk)]

Comment on attachment 9297067 [details]
Bug 1792362 - Get All New Messages button should work for nntp servers. r=mkmelin

[Triage Comment]
Approved for esr102

Comment 17

[Rob Lemley [:rjl]]

Thunderbird 102.4.2:
https://hg.mozilla.org/releases/comm-esr102/rev/482675ab9b42
https://hg.mozilla.org/releases/comm-esr102/rev/31af0add7293