83486 - News doesn't try to reload a message if an error occurs on the first try

Status: VERIFIED FIXED

(MailNews Core :: Networking: NNTP, defect, P2)

Description

[Skewer]

Procedure: Click on a message, and hit the stop button before it finishes
loading. Go to another message and come back, or open the message in a new window.

Expected: News tries to download the message again, displaying it when it loads.

Actual: News never tries to download the message again, so the stopped message
is always displayed as blank (or partial?) until Mozilla is shut down and restarted.

Build: 2001053004 Win98

Comment 1

[Skewer]

Keywording...

Comment 2

[[e-mail] (gone - use [e-mail] instead)]

.

Comment 3

[Skewer]

stephend: Please post relevant comments when modifying my bugs. Periods are
still spam, anyway.

Comment 4

[[e-mail] (gone - use [e-mail] instead)]

I wasn't spamming, I was re-assigning QA contact to myself.  If you don't like
e-mail notification, you can turn that off in your bugzilla prefs.

Comment 5

[[e-mail] (gone - use [e-mail] instead)]

David might be able to shed some insight on this.  We're not loading the full
message into our cache, so when we return to it, we're not finding it, and have
to load it again, correct, David?

Comment 6

[David :Bienvenu]

the problem is that we've left the half-downloaded message in the memory cache -
we need to remove the message from the memory cache if the download failed or
was interrupted. We do this for imap in some situations right now - I can find
the relevant code

Comment 7

[David :Bienvenu]

Here's the imap code we run when finishing a url - it gets the channel and
checks its status - if the channel indicates a failure, we remove the mem cache
entry for the url, if any. We should do something similar for news.

     if (NS_SUCCEEDED(aImapUrl->GetMockChannel(getter_AddRefs(mockChannel))) &&
mockChannel)
      {
        nsCOMPtr<nsIRequest> request = do_QueryInterface(mockChannel);
        if (!request)
          return NS_ERROR_FAILURE;
        request->GetStatus(&rv);
        if (!NS_SUCCEEDED(rv))
        {
          nsresult res;
          removeUrlFromQueue = PR_TRUE;

          mockChannel->Close(); // try closing it to get channel listener nulled
out.

          if (aMailNewsUrl)
          {
            nsCOMPtr<nsICacheEntryDescriptor>  cacheEntry;
            res = aMailNewsUrl->GetMemCacheEntry(getter_AddRefs(cacheEntry));
            if (NS_SUCCEEDED(res) && cacheEntry)
              cacheEntry->Doom();
          }
        }
      }

Comment 8

[scottputterman]

moving to 0.9.2 and reassigning to cavin.

Comment 9

[Cavin Song]

The patch I'm working on looks good so far. Will submit patch tomorrow.

Comment 10

[scottputterman]

adding PDT+

Comment 11

[Cavin Song]

Attachment: Fixed the problem.

Comment 12

[Scott MacGregor]

looks good to me. r=mscott

Comment 13

[David :Bienvenu]

sr=bienvenu

Comment 14

[Skewer]

Adding patch keyword.

Comment 15

[Christopher Blizzard (:blizzard)]

a=blizzard on behalf of drivers for the trunk

Comment 16

[Cavin Song]

Remove incomplete cache entry in OnStopReuqest() when failed.

Comment 17

[[e-mail] (gone - use [e-mail] instead)]

Verified Fixed on : 
Mac OS 9.1 : 2001-06-15-08
RedHat 7.1 : 2001-06-15-08
Windows 2K : 2001-06-15-04

When I click on a large message, press the 'Stop' button, and then either
double-click to load the message in a stand-alone window, or simply single-click
to load in the current message pane, the message loads fine.  

Comment 18

[Skewer]

I saw this again today. I tried to open a message and got an error box that said
"200 News Server Ready" (???) and the message never downloaded, even if I tried
to reload it.

Comment 19

[David :Bienvenu]

Skewer, did you press stop? If not, that's a different bug.

Comment 20

[Skewer]

It seems like it is caused by the same cache problem. Changing summary.

Comment 21

[bclinger]

There normally is not even an error message generated - there are times it
simply will not display the message. Go back and forth and then try it again and
it normally will show it. It also is not a funciton of how fast the user is
clicking.

Comment 22

[Skewer]

No, I have determined that when a message is not loaded properly because of the
underlying problem in this bug, it never reloads until the entire browser (not
just that one window) is restarted. Of course, it should reload the message
every time it's loaded until it is successfully downloaded.

Comment 23

[Håkan Waara]

mozilla-0.9.2 train is long gone. unsetting Target Milestone for cavin.

Comment 24

[Jaime Rodriguez, Jr.]

Is this fixed on the 094 branch?

Comment 25

[Cavin Song]

The new problem reported by Skewer on 2001-08-12 23:28 has not been addressed.
The original problem was resolved and is on 094 branch.  

Comment 26

[Scott MacGregor]

can you guys file a new bug on the additional scenario Skewer found and let's
reclose this one. 

Comment 27

[Cavin Song]

The problem reported by Skewer is recorded in bug #104983.  Closing this bug.

Comment 28

[[e-mail] (gone - use [e-mail] instead)]

From http://bugzilla.mozilla.org/queryhelp.cgi#status

    * VERIFIED- QA has looked at the bug and the resolution and agrees that the
appropriate action has been taken.
    * CLOSED - The bug is considered dead, the resolution is correct, and the
product the bug has been reported against is terminated or shipped. Any zombie
bugs who choose to walk the earth again must do so by becoming REOPENED. This
state is rarely ever used.


Cavin, not your fault, you didn't know.  But I'll go ahead and re-open, then
resolve it fixed, then verify.

Comment 29

[[e-mail] (gone - use [e-mail] instead)]

resolving

Comment 30

[[e-mail] (gone - use [e-mail] instead)]

...and verified.