Closed Bug 1795547 Opened 2 months ago Closed 1 month ago

File > Offline > Download/Sync Now ... doesn't download messages before going offline in NNTP-JS

Categories

(MailNews Core :: Networking: NNTP, defect)

Thunderbird 102
defect

Tracking

(thunderbird_esr102 fixed, thunderbird107 fixed)

RESOLVED FIXED
108 Branch
Tracking Status
thunderbird_esr102 --- fixed
thunderbird107 --- fixed

People

(Reporter: b4, Assigned: rnons)

References

(Blocks 1 open bug, Regression)

Details

(Keywords: regression)

Attachments

(4 files, 1 obsolete file)

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

Steps to reproduce:

File > Offline > Download/Sync Now ... doesn't download messages before going offline in NNTP-JS

Have a newsgroup that is selected for offline use.
Go offline using "File > Offline > Download/Sync Now ..." and select newsgroup messages.

Actual results:

After going offline, not all newsgroups have been downloaded. In fact, when switching to the old C++ module, it's quite visible in the status bar that messages are downloaded, not so for the JS module.

Expected results:

Messages should have been downloaded for offline use.

Blocks: tb102found
Keywords: regression

(In reply to b4 from comment #0)

After going offline, not all newsgroups have been downloaded. In fact, when switching to the old C++ module, it's quite visible in the status bar that messages are downloaded, not so for the JS module.

Confirmed for Release and Daily.

Status: UNCONFIRMED → NEW
Ever confirmed: true
Flags: needinfo?(remotenonsense)

OK, I forgot to implement nsINntpService.downloadNewsgroupsForOffline, will rewrite nsNewsDownloader.cpp in JS.

Assignee: nobody → remotenonsense
Status: NEW → ASSIGNED
Flags: needinfo?(remotenonsense)
Target Milestone: --- → 108 Branch

Pushed by mkmelin@iki.fi:
https://hg.mozilla.org/comm-central/rev/bf9dd95a17f3
Support downloading all news before going offline. r=mkmelin

Status: ASSIGNED → RESOLVED
Closed: 1 month ago
Resolution: --- → FIXED
Regressed by: nntp-js
Attached image news-setup.png

Can you please look at this again, it's not quite working like in the C++ module.

  1. First and foremost, the "Work offline once download and/or sync is complete" option doesn't work.
  2. Secondly, at least from the status bar display it appears to be downloading newsgroups which weren't selected for offline use.
  3. It also appears to re-download certain messages, maybe those are expired articles, there are 4 on a.d.test. That also happens in the C++ module.
  4. Further to one, when going offline because automatically going offline doesn't work, there is a whole lot of activity in NGs not selected for offline use. According to the status bar display, all the headers appear to be downloaded again.

Another issue which is unrelated to this bug but related to offline use in general is that clicking a NG which wasn't selected for offline use while in offline use causes an hourglass to be displayed.

In the current state, offline news reading isn't really usable.

If you want to reproduce our testing, see the subscribed newsgroups in the attached screenshot.

Flags: needinfo?(remotenonsense)

"Work offline once download and/or sync is complete" seems to work for me, please try again.

2 and 4 seems to be the same, the new patch should fix them.

Another issue which is unrelated to this bug but related to offline use in general is that clicking a NG which wasn't selected for offline use while in offline use causes an hourglass to be displayed.

Let me fix this separately. Thanks.

Flags: needinfo?(remotenonsense)

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

"Work offline once download and/or sync is complete" seems to work for me, please try again.

Yes, this works on 102 with part 1 applied (attachment 9300953 [details] [diff] [review]) and Daily with a small setup, one small newsgroup. It likely didn't work on our test setup (comment #6) since downloading of the NGs which weren't meant to be downloaded never ended (issue 2 & 4). We'll test again with part 2 applied.

2 and 4 seems to be the same, the new patch should fix them.

Thank you.

Another issue which is unrelated to this bug but related to offline use in general is that clicking a NG which wasn't selected for offline use while in offline use causes an hourglass to be displayed.

Let me fix this separately. Thanks.

Will you file a bug?

(In reply to b5 from comment #9)

We'll test again with part 2 applied.

This appears to be working now with 1. also resolved. Merged ESR 102 version attached for your convenience.

Attachment #9300953 - Attachment is obsolete: true
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Blocks: 1798597

Pushed by mkmelin@iki.fi:
https://hg.mozilla.org/comm-central/rev/4de95d512152
Download articles for newsgroup set for offline use only. r=mkmelin

Status: REOPENED → RESOLVED
Closed: 1 month ago1 month ago
Resolution: --- → FIXED

Comment on attachment 9300904 [details]
Bug 1795547 - Support downloading all news before going offline. r=mkmelin

[Triage Comment]
Approved for beta in consideration for esr102. Note to uplifter: two changesets to uplift.

[Approval Request Comment]
Regression caused by (bug #): nntp-js
User impact if declined: NNTP messages are not synced prior to going offline
Testing completed (on c-c, etc.): By Betterbird devs
Risk to taking this patch (and alternatives if risky):

Attachment #9300904 - Flags: approval-comm-esr102?
Attachment #9300904 - Flags: approval-comm-beta+
Attachment #9300904 - Flags: approval-comm-esr102?

Comment on attachment 9301291 [details] [diff] [review]
1795547-download-news-before-offline.patch (ESR 102 version, merge of both patches)

[Approval Request Comment]
Regression caused by (bug #): nntp-js
User impact if declined: NNTP messages are not synced prior to going offline
Testing completed (on c-c, etc.): By Betterbird devs
Risk to taking this patch (and alternatives if risky):

Ping - any objections? This patch is from an outside contributor and should be reviewed again.

Attachment #9301291 - Flags: approval-comm-esr102?

Comment on attachment 9301291 [details] [diff] [review]
1795547-download-news-before-offline.patch (ESR 102 version, merge of both patches)

[Triage Comment]
Approved for esr102

Attachment #9301291 - Flags: approval-comm-esr102? → approval-comm-esr102+
Attachment #9301291 - Flags: review+
You need to log in before you can comment on or make changes to this bug.