NNTP: Incomplete loading of multiple inline images/content

VERIFIED FIXED

Status

MailNews Core
Networking: NNTP
--
major
VERIFIED FIXED
10 years ago
10 years ago

People

(Reporter: JoeS1, Assigned: jcranmer)

Tracking

({regression})

Trunk
x86
All
regression
Bug Flags:
blocking-thunderbird3 -

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments)

(Reporter)

Description

10 years ago
Steps:
Open a Newgroups message with more than 2 inline images.
Note that only the first 2 image display

You may have to open the same message more than once to see the problem.

Regression Range:
OK
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1b3pre) Gecko/20081218 Lightning/1.0pre Shredder/3.0b2pre ID:20081218031250

Bad
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1b3pre) Gecko/20081219 Lightning/1.0pre Shredder/3.0b2pre ID:20081219032723

The same identical message will display fine when opened from a local folder.

BTW: Changing Max_Cached_connections pref to a higher number does not seem to help.

Updated

10 years ago
Duplicate of this bug: 470685
If bug 470685 (a Linux bug) is a dupe, then this bug is not only seen on Windows.
OS: Windows XP → All
(Reporter)

Comment 4

10 years ago
Created attachment 354562 [details]
nntp log

The log refers to the testcase post.
The post is multipart/related with 5 parts.
After the 2nd part rendered, TB spins with "loading nessage in the status bar.
I closed the window at that point.
This makes current nightlies pretty much unusable for viewing newsgroup posts of this type.

Comment 5

10 years ago
(In reply to comment #4)

> This makes current nightlies pretty much unusable for viewing newsgroup posts
> of this type.

Backing out as mentioned in Bug 470685 helps. I am doing so for my private builds until 471131 is fixed.
(Reporter)

Updated

10 years ago
Flags: blocking-thunderbird3?
(Reporter)

Comment 6

10 years ago
(In reply to comment #0)

> BTW: Changing Max_Cached_connections pref to a higher number does not seem to
> help.

Well, it does help if you discover the correct pref in configedit
mail.server.server5.max_cached_connections (In my case)

IMAP default is set to 10
Why should we default NNTP to anything less.

Comment 7

10 years ago
imap default is 5.

For news, building up the state for a particular folder/group is a lot cheaper than it is for imap, so there's much less incentive to cache connections.
(Assignee)

Comment 8

10 years ago
(In reply to comment #6)
> IMAP default is set to 10
> Why should we default NNTP to anything less.

NNTP servers (especially ISP servers) very frequently throttle the number of connections; 2 is a common maximum, although premium services may have more.

And the issue here is orthogonal: NNTP is not IMAP, so it's far better to ask why one should use the same settings for IMAP and NNTP.
(Reporter)

Comment 9

10 years ago
But why should the Max# connections preclude closing the connection, and re-opening to get the remaining parts of the post.
Maybe we are not closing the connections at all ??
(Reporter)

Comment 10

10 years ago
netstat shows that a connection is opened for each part of a multi-part message.
The connections are not closed at the completion of each part, nor at the end
of the "body load"

I don't remember how to pipe netstat results to a file to document this.

If mail.server.serverX.max_cached_connections remains at a default of 2
that means that you can never view a multipart post of more than 2 parts.

If this is going to hold, then I think we must add UI to make this more
discoverable.
doesn't block tb3.
Flags: blocking-thunderbird3? → blocking-thunderbird3-
(Reporter)

Comment 12

10 years ago
(In reply to comment #11)
> doesn't block tb3.

Well, it _is_ a recent regression caused by the bug 66150 "fix"
Certainly would impact newsgroup users coming over from tb2 that will be
quite surprised by the change.

The work-around pref (mail.server.serverX.max_cached_connections)
is not very discoverable at all.
(Assignee)

Comment 13

10 years ago
I've been running through the code with a debugger and debug statements at various points and I've found the problem.

It turns out that, because of some ancient bug, the NNTP protocol object fails to work properly when it's loading an article or part from the cache. Since bug 66150, we've tried to reuse these unusable protocol objects.

I should be able to get this working within the next week.
Assignee: nobody → Pidgeot18
Status: NEW → ASSIGNED
(Assignee)

Comment 14

10 years ago
Created attachment 356342 [details] [diff] [review]
Patch, version 1

Note to self: do not run make check when a patch known to break make check is applied.

As noted in the previous comment, the cause of the problem is that AsyncOpen fails to run the next URL in some cases, namely when reading offline or from the memory cache. This patch ensures that the next URL runs in these cases.

I've also cleaned up the comments in AsyncOpen (no one's really touched that since mscott wrote that 8 years ago), and added a log note when the file is being read from offline.
Attachment #356342 - Flags: superreview?(bienvenu)
Attachment #356342 - Flags: review?(bienvenu)

Comment 15

10 years ago
(In reply to comment #14)

> Created an attachment (id=356342) [details]

FWIW, this patch works for me.

Updated

10 years ago
Attachment #356342 - Flags: superreview?(bienvenu)
Attachment #356342 - Flags: superreview+
Attachment #356342 - Flags: review?(bienvenu)
Attachment #356342 - Flags: review+
(Assignee)

Comment 16

10 years ago
Pushed as changeset 57470c8e8c12.
Status: ASSIGNED → RESOLVED
Last Resolved: 10 years ago
Resolution: --- → FIXED
(Reporter)

Comment 17

10 years ago
Checked with hourly:
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1b3pre) Gecko/20090117 Lightning/1.0pre Shredder/3.0b2pre ID:20090117195944

Another ancient problem has been fixed:
Saved images are now retrieved from the memory cache, and not re-downloaded.
(although the memory cache default might have to be bumped up)

Nice to see newsgroups users needs not regarded as "edge cases"

Thanks to Joshua and David
Status: RESOLVED → VERIFIED

Comment 18

10 years ago
(In reply to comment #16)
> Pushed as changeset 57470c8e8c12.

What I am going to test is if this also resolves the constant refetching of attachments to news when each attachment is opened sequentialy.

Comment 19

10 years ago
I did not find enough test massages to be satisfied with the quality of my testing.  Yet I found this fix seems to have fixed the refetch from server for each attachment opened by a viewer/handler.

Now we need to find the related bugs to process for closure.

Comment 20

10 years ago
More testing and I am rescinding comment #19. This fix has no effect on the case where an posting has several binary attachments. Selecting each to open in it's associated viewer (tested with Windows Fontviewer and *.TTF files) resulted in the old behavior of reloading from the server for each attachment viewed. Checking memory cache with about:cache(memory) showed it was not being used.
Product: Core → MailNews Core
You need to log in before you can comment on or make changes to this bug.