52667 - Page doesn't load after cookie warning box is displayed [mac+linux]

Status: VERIFIED FIXED

(Core :: Networking: Cookies, defect, P2)

Description

[Dave]

Build 2000091408/Linux

I tested with a brand new ~/.mozilla directory and, upon logging into my account
at http://mail.yahoo.com, I observed that although mozilla now remembers cookie
decisions, that's all it does. It no longer loads the page. To reproduce:

1.) Start with a clean ~/.mozilla directory (deleting the cookie related files
may also work).
2.) set edit->prefs->advanced->cookies to accept all cookies but warn first.
3.) go to http://mail.yahoo.com and wait for the cookie warning to pop up.
4.) Check "remember this decision" and hit OK button.
5.) The cookie is stored and the site added to the "cookie sites" list, but the
browser doesn't go to the yahoo mail page until one hits enter on the url bar a
second time.

This bug isn't specific to yahoo mail. I've also been able to reproduce it at
http://www.cnn.com and http://bugzilla.mozilla.org

Comment 1

[Dave]

Appears to be Linux only since I can't reproduce on 2000091408/NT

Comment 2

[Tom Everingham]

confirming - also seeing at www.sun.com and a few others.  Happens frequently.

Comment 3

[Stephen P. Morse]

To reproduce this, and be able to keep reproducing it without having to erase 
cookies, modify the procedure to the following:

1. (First time only)  Remove all cookies and all sites that can/cannot set 
cookies.  Do this from cookie manager 
(tasks->privacy->cookie-manager->view-cookies).

2. (First time only)  Set prefs to enable-cookies and 
warn-before-accepting-cookies.

3. Go to http://mail.yahoo.com

4. Cookie warning box appears.  Uncheck "remember this decision" and click NO 
button.

5. Browser does not go to yahoo mail page.

Comment 4

[Stephen P. Morse]

The following assertion is occuring which I suspect might be causing the problem 
(this assertion does not occur on windows, in which case the browser does indeed 
go to the yahoo mail page).

###!!! ASSERTION: NS_ENSURE_TRUE(aListener) failed: 'aListener', file nsHTTPChan
nel.cpp, line 1266
###!!! Break: at file nsHTTPChannel.cpp, line 1266

That assertion is occuring in the method nsHTTPChannel::CacheReceiveResponse.  
Copying Gagan and Neeti to see if they can explain this.

Comment 5

[Stephen P. Morse]

Am no longer able to reproduce this bug using mail.yahoo.com because that site 
times out today for some reason (it comes up fine in a 4.x browser).  However I 
was still able to reproduce it using www.cnn.com.  My build is from September 
15.

Then I pulled and build a new tree as of noon today (September 19).  With that 
build I no longer have the problem accessing www.cnn.com.  So I'm going to go 
out on a limb and mark this as works-for-me.  Please re-open immediately if you 
are still seeing the problem.

Comment 6

[Dave]

Hmm.. I'm definitely still seeing this on 2000091908/Linux which, I believe, is
the most recent Linux build. It's been annoying me all day. I'm seeing it at
cnn.com too... (renamed my cookperm.txt file to reproduce that one).

Comment 7

[Stephen P. Morse]

OK, I tried it again and this time mail.yahoo.com does not time out so I was 
able to test it.  And you are correct, the problem is still present there.

However I retested www.cnn.com and that site is working fine with no problem.  
And it did indeed have this problem in my September 15 build.

Comment 8

[Dave]

I attempted to reproduce the bug at cnn.com by deleting it out of my cookie
sites list and was unable to. However, I can still reproduce it by renaming my
cookperm.txt file to cookperm.txt.bak and forcing the browser to start over.

I can repro the bug at mail.yahoo.com simply by removing the set cookies and the
entry in the cookies sites list.

The only difference I can see between these two cases is that I'm permitting
yahoo mail to set cookies while I'm refusing them from cnn.com. However, I also
attempted to answer yes when going to cnn.com and still was not able to
reproduce the bug unless I renamed my cookperm.txt file.

Comment 9

[Phil Peterson]

PDT thinks this is a P2

Comment 10

[Stephen P. Morse]

Latest update.  I did some investigation and compared it to the behavior on 
windows and also to the behavior when the cookie warning dialog does not come 
up.  Here is what I discovered.

1. Windows and linux without cookie warning dialog:

   -- nsHTTPChannel::AsyncRead invoked
      sets mResponseDataListener to a value

   -- nsHTTPChannel::ProcessStatusCode invoked
      invokes nsHTTPCacheReceivedResponse passing mResponseDataListener as arg 
      nsHTTPChannel::CacheReceivedResponse would assert if arg is null

   -- nsHTTPChannel::ResponseCompleted invoked
      sets mResponseDataListener to null

2. Windows with cookie warning dialog:

   -- nsHTTPChannel::AsyncRead invoked
      sets mResponseDataListener

   -- Cookie warning dialog occurs, click NO

   -- nsHTTPChannel::ProcessStatusCode invoked
      invokes nsHTTPCacheReceivedResponse passing mResponseDataListener as arg 
      nsHTTPChannel::CacheReceivedResponse would assert if arg is null

   -- nsHTTPChannel::ResponseCompleted invoked
      sets mResponseDataListener to null

3. Linux with cookie warning dialog:

   -- nsHTTPChannel::AsyncRead invoked
      sets mResponseDataListener

   -- nsHTTPChannel::ResponseCompleted invoked
      sets mResponseDataListener to null

   -- Cookie warning dialog occurs, click NO

   -- nsHTTPChannel::ProcessStatusCode invoked
      invokes nsHTTPCacheReceivedResponse passing mResponseDataListener as arg 
      nsHTTPChannel::CacheReceivedResponse asserts because arg is null

Note that in case 3 the ResponseCompleted occurs before the ProcessStatusCode 
which results in the listener argument becoming null and eventually causing 
CacheReceivedResponse to assert.

It looks like the problem is that the bringing up of the cookie warning box is 
forcing a premature call to ResponseCompleted.  Now to figure out why. 

Comment 11

[Stephen P. Morse]

Just to clarify my comments above.  The assert is obviously not the problem, but 
it is an indication that things have already gone bad.  In particular, the 
assert indicates that data has arrived for a channel that has already 
received a ResponseCompleted.  And the ResponseCompleted is occuring when the 
cookie warning box is put up.

The consequence of this is that the arriving data probably gets lost and that 
explains why the page never gets loaded.

Comment 12

[Stephen P. Morse]

This might be a dup of 52397.  Would like to try out gagan's fix for that bug to 
see if it fixes this one.

Comment 13

[Stephen P. Morse]

Now here's a very interesting stack trace.  It confirms that the bringing up of 
the cookie warning box is triggering the response-completed message although the 
data has not yet been read.

This stack trace is taken at the point that ResponseCompleted is being entered.  
Note that it shows the call to bring up the cookie warning box which gets to 
nsXULWindow.cpp line 235 which in turns dispatches the event for 
ResponseCompleted.

(gdb) backtrace
#0  nsHTTPChannel::ResponseCompleted (this=0x421f2780, aListener=0x0,
    aStatus=0, aStatusArg=0x4017a7d0) at nsHTTPChannel.cpp:1897
#1  0x40a01511 in nsHTTPServerListener::OnStopRequest (this=0x4230be30,
    channel=0x421e2904, i_pContext=0x421f2780, i_Status=0,
    aStatusArg=0x4017a7d0) at nsHTTPResponseListener.cpp:728
#2  0x40995b09 in nsOnStopRequestEvent::HandleEvent (this=0x421b8d68)
    at nsAsyncStreamListener.cpp:301
#3  0x409950b7 in nsStreamListenerEvent::HandlePLEvent (aEvent=0x421b8da0)
    at nsAsyncStreamListener.cpp:97
#4  0x401244be in PL_HandleEvent (self=0x421b8da0) at plevent.c:575
#5  0x401242dc in PL_ProcessPendingEvents (self=0x80a9e48) at plevent.c:508
#6  0x40126129 in nsEventQueueImpl::ProcessPendingEvents (this=0x80a9e20)
    at nsEventQueue.cpp:356
#7  0x40c5ca74 in event_processor_callback (data=0x80a9e20, source=8,
    condition=GDK_INPUT_READ) at nsAppShell.cpp:158
#8  0x40c5c6af in our_gdk_io_invoke (source=0x41b64798, condition=G_IO_IN,
    data=0x41f0cf80) at nsAppShell.cpp:58
#9  0x40e2352a in g_io_unix_dispatch () from /usr/lib/libglib-1.2.so.0
#10 0x40e24be6 in g_main_dispatch () from /usr/lib/libglib-1.2.so.0
#11 0x40e251a1 in g_main_iterate () from /usr/lib/libglib-1.2.so.0
#12 0x40e25254 in g_main_iteration () from /usr/lib/libglib-1.2.so.0
#13 0x40c5d217 in nsAppShell::DispatchNativeEvent (this=0x421a8408,
    aRealEvent=0, aEvent=0x0) at nsAppShell.cpp:372
---Type <return> to continue, or q <return> to quit---
#14 0x4073d387 in nsXULWindow::ShowModal (this=0x421c1910)
    at nsXULWindow.cpp:235
#15 0x4074879e in nsWebShellWindow::ShowModal (this=0x421c1910)
    at nsWebShellWindow.cpp:1141
#16 0x407393c9 in nsChromeTreeOwner::ShowModal (this=0x4234bfd8)
    at nsChromeTreeOwner.cpp:181
#17 0x4054d0dc in GlobalWindowImpl::OpenInternal (this=0x41f13810,
    cx=0x41f1c998, argv=0x421bf310, argc=4, aDialog=1, aReturn=0xbfffed44)
    at nsGlobalWindow.cpp:3056
#18 0x40546f91 in GlobalWindowImpl::OpenDialog (this=0x41f13810,
    cx=0x41f1c998, argv=0x421bf310, argc=4, aReturn=0xbfffed44)
    at nsGlobalWindow.cpp:1999
#19 0x407551a4 in nsCommonDialogs::DoDialog (this=0x8293550,
    inParent=0x41f13814, ioParamBlock=0x421e51b0,
    inChromeURL=0x4077b6a0 "chrome://global/content/commonDialog.xul")
    at nsCommonDialogs.cpp:453
#20 0x4075460a in nsCommonDialogs::UniversalDialog (this=0x8293550,
    inParent=0x41f13814, inTitleMessage=0x0, inDialogTitle=0x42334920,
    inMsg=0x421bf230, inCheckboxMsg=0x42334940, inButton0Text=0x423771f8,
    inButton1Text=0x42377208, inButton2Text=0x0, inButton3Text=0x0,
    inEditfield1Msg=0x0, inEditfield2Msg=0x0, inoutEditfield1Value=0x0,
    inoutEditfield2Value=0x0, inIConURL=0x42377170,
    inoutCheckboxState=0xbffff038, inNumberButtons=2, inNumberEditfields=0,
---Type <return> to continue, or q <return> to quit---
    inEditField1Password=0, outButtonPressed=0xbfffeff4)
    at nsCommonDialogs.cpp:257
#21 0x4074b44f in nsDOMWindowPrompter::UniversalDialog (this=0x423a7390,
    inTitleMessage=0x0, inDialogTitle=0x42334920, inMsg=0x421bf230,
    inCheckboxMsg=0x42334940, inButton0Text=0x423771f8,
    inButton1Text=0x42377208, inButton2Text=0x0, inButton3Text=0x0,
    inEditfield1Msg=0x0, inEditfield2Msg=0x0, inoutEditfield1Value=0x0,
    inoutEditfield2Value=0x0, inIConURL=0x42377170,
    inoutCheckboxState=0xbffff038, inNumberButtons=2, inNumberEditfields=0,
    inEditField1Password=0, outButtonPressed=0xbfffeff4)
    at nsWebShellWindow.cpp:2116
#22 0x41cd3f00 in nsSingleSignOnPrompt::UniversalDialog (this=0x423a7400,
    titleMessage=0x0, dialogTitle=0x42334920, text=0x421bf230,
    checkboxMsg=0x42334940, button0Text=0x423771f8, button1Text=0x42377208,
    button2Text=0x0, button3Text=0x0, editfield1Msg=0x0, editfield2Msg=0x0,
    editfield1Value=0x0, editfield2Value=0x0, iconURL=0x42377170,
    checkboxState=0xbffff038, numberButtons=2, numberEditfields=0,
    editField1Password=0, buttonPressed=0xbfffeff4) at nsWalletService.cpp:513
#23 0x4198bc33 in cookie_CheckConfirmYN (aPrompter=0x423a7400,
    szMessage=0x421bf230, szCheckMessage=0x42334940, checkValue=0xbffff038)
    at nsCookie.cpp:267
#24 0x4198d30c in permission_Check (aPrompter=0x423a7400,
    hostname=0x421bf088 ".yahoo.com", type=0, warningPref=1,
---Type <return> to continue, or q <return> to quit---
    message=0x421bf230) at nsCookie.cpp:867
#25 0x4198ee66 in cookie_SetCookieString (
    curURL=0x421bf000 "http://mail.yahoo.com/", aPrompter=0x423a7400,
    setCookieHeader=0x421bef78 "B", timeToExpire=1271361005)
    at nsCookie.cpp:1646
#26 0x4198f4ed in COOKIE_SetCookieStringFromHttp (
    curURL=0x421bf000 "http://mail.yahoo.com/",
    firstURL=0x421bf020 "http://mail.yahoo.com/", aPrompter=0x423a7400,
    setCookieHeader=0x421bef78 "B",
    server_date=0x421befd8 "Thu, 21 Sep 2000 12:27:46 GMT")
    at nsCookie.cpp:1823
#27 0x4198b079 in nsCookieService::SetCookieStringFromHttp (this=0x4231d0d8,
    aURL=0x421f2680, aFirstURL=0x421f2680, aPrompter=0x423a7400,
    aCookie=0x421bef78 "B",
    aExpires=0x421befd8 "Thu, 21 Sep 2000 12:27:46 GMT")
    at nsCookieService.cpp:137
#28 0x41993f72 in nsCookieHTTPNotify::AsyncExamineResponse (this=0x81dbe98,
    aContext=0x421f2780) at nsCookieHTTPNotify.cpp:257
#29 0x401429c5 in XPTC_InvokeByIndex (that=0x81dbe98, methodIndex=4,
    paramCount=1, params=0x423a5ad0) at xptcinvoke_unixish_x86.cpp:134
#30 0x4012b640 in EventHandler (self=0x421beeb0) at nsProxyEvent.cpp:506
#31 0x4012b436 in nsProxyObject::Post (this=0x41f2eaa8, methodIndex=4,
    methodInfo=0x423aa764, params=0xbffff43c, interfaceInfo=0x80c4c08)
---Type <return> to continue, or q <return> to quit---
    at nsProxyEvent.cpp:449
#32 0x4012da01 in nsProxyEventObject::CallMethod (this=0x41b88fb0,
    methodIndex=4, info=0x423aa764, params=0xbffff43c)
    at nsProxyEventObject.cpp:429
#33 0x40142f2d in PrepareAndDispatch (self=0x41b88fb0, methodIndex=4,
    args=0xbffff4f4) at xptcstubs_unixish_x86.cpp:80
#34 0x4014304a in nsXPTCStubBase::Stub4 (this=0x41b88fb0)
    at ../../../../../../dist/include/xptcstubsdef.inc:6
#35 0x409f6e4a in nsHTTPChannel::OnHeadersAvailable (this=0x421f2780)
    at nsHTTPChannel.cpp:1981
#36 0x409f85f8 in nsHTTPChannel::FinishedResponseHeaders (this=0x421f2780)
    at nsHTTPChannel.cpp:2237
#37 0x40a01fc1 in nsHTTPServerListener::FinishedResponseHeaders (
    this=0x4230be30) at nsHTTPResponseListener.cpp:1039
#38 0x40a003f9 in nsHTTPServerListener::OnDataAvailable (this=0x4230be30,
    channel=0x421e2904, context=0x421f2780, i_pStream=0x421b9308,
    i_SourceOffset=0, i_Length=5823) at nsHTTPResponseListener.cpp:427
#39 0x40995e2f in nsOnDataAvailableEvent::HandleEvent (this=0x421b8a60)
    at nsAsyncStreamListener.cpp:400
#40 0x409950b7 in nsStreamListenerEvent::HandlePLEvent (aEvent=0x421b8a88)
    at nsAsyncStreamListener.cpp:97
#41 0x401244be in PL_HandleEvent (self=0x421b8a88) at plevent.c:575
#42 0x401242dc in PL_ProcessPendingEvents (self=0x80a9e48) at plevent.c:508
---Type <return> to continue, or q <return> to quit---
#43 0x40126129 in nsEventQueueImpl::ProcessPendingEvents (this=0x80a9e20)
    at nsEventQueue.cpp:356
#44 0x40c5ca74 in event_processor_callback (data=0x80a9e20, source=8,
    condition=GDK_INPUT_READ) at nsAppShell.cpp:158
#45 0x40c5c6af in our_gdk_io_invoke (source=0x41b64798, condition=G_IO_IN,
    data=0x41f0cf80) at nsAppShell.cpp:58
#46 0x40e2352a in g_io_unix_dispatch () from /usr/lib/libglib-1.2.so.0
#47 0x40e24be6 in g_main_dispatch () from /usr/lib/libglib-1.2.so.0
#48 0x40e251a1 in g_main_iterate () from /usr/lib/libglib-1.2.so.0
#49 0x40e25341 in g_main_run () from /usr/lib/libglib-1.2.so.0
#50 0x40d4f209 in gtk_main () from /usr/lib/libgtk-1.2.so.0
#51 0x40c5d16a in nsAppShell::Run (this=0x8113fb0) at nsAppShell.cpp:335
#52 0x40744164 in nsAppShellService::Run (this=0x8109760)
    at nsAppShellService.cpp:407
#53 0x805576f in main1 (argc=1, argv=0xbffffc04, nativeApp=0x0)
    at nsAppRunner.cpp:958
#54 0x8055e3e in main (argc=1, argv=0xbffffc04) at nsAppRunner.cpp:1139
#55 0x403791eb in __libc_start_main (main=0x8055c8c <main>, argc=1,
    argv=0xbffffc04, init=0x8050914 <_init>, fini=0x80627bc <_fini>,
    rtld_fini=0x4000a610 <_dl_fini>, stack_end=0xbffffbfc)
    at ../sysdeps/generic/libc-start.c:90
(gdb)                                                                          

Comment 14

[Stephen P. Morse]

Assigning to dougt at his request.

Comment 15

[Stephen P. Morse]

.

Comment 16

[Doug Turner (:dougt)]

Attachment: Nested Event Queue fix.

Comment 17

[Doug Turner (:dougt)]

Steve, I have verified this patch does fix the problem.  Could you verify this
as well?

Comment 18

[Stephen P. Morse]

Yes, it fixes it for me too.

Comment 19

[Doug Turner (:dougt)]

I sent out the patch last night to a bunch of people for review.  Once I get a
reviewer or two, and a approval, I will check it in.

Comment 20

[Stephen P. Morse]

*** Bug 53877 has been marked as a duplicate of this bug. ***

Comment 21

[Stephen P. Morse]

Bug 53877, which I just duped to this one, indicates that this problem occurs on 
a mac as well.  So only win32 is immune from this.

Updating summary to be a bit more meaningful.

Comment 22

[Stephen P. Morse]

*** Bug 54180 has been marked as a duplicate of this bug. ***

Comment 23

[Phil Peterson]

Not holding PR3 for this. Marking nsbeta3- but adding rtm nomination so we 
remember to add the patch on the branch before Seamonkey RTM.

Comment 24

[Stephen P. Morse]

I haven't tried this out, but I would imagine that this will occur in many more 
situations that just the cookie nag box.  Probably whenever any dialog appears 
during page loading.  For example http authentication and ftp authentication.

So here we have a bug that will prevent pages from being loaded on two of our 
three platforms in any of the following situations:

   - page sets cookies and user has cookie-warning pref set
   - page requires http authentication
   - page requires ftp authentication

And there are probably other situations as well that we don't know about.  How 
can we ship out a beta that fails to load pages in so many common situations?

This is a regression which will make beta3 considerably worse (in the users 
eyes) than beta2.  And dougt has a fix in hand.  Renominating for beta3 
consideration.

Comment 25

[Daniel Veditz [:dveditz]]

Marking rtm+, don't see the point of re-marking nsbeta3+ because given the 
schedule the PDT probably wouldn't see it in time. Steve (and/or Dout?), you're 
going to have to lobby them directly, either 4pm daily in Star Trek or via mail 
to [e-mail], explaining why a low-level event Q change with no time for 
"baking" is not scarier than a known set of problems.

Might help if you also demonstrated sites that could not be accessed at all due 
to the authentication dialog. The cookie problem has an easy workaround (turn 
off the warnings) which is I'm sure why they are rejecting this for beta3

If this is not accepted we should release note the workaround.

"On Linux and Mac you may have problems loading sites with the 'warn on cookie' 
option turned on. If you encounter this problem go into preferences and turn 
off the cookie warning feature."

Comment 26

[Stephen P. Morse]

Hmm, this is now working.  dougt, did you ever get your patch checked in?

Comment 27

[Doug Turner (:dougt)]

I did not check it in.

Comment 28

[Stephen P. Morse]

Reopening in that case so that you can get the fix checked in.

Comment 29

[J. Paul Reed [:preed]]

Just FYI, this is now broken again in 2000092721.

Comment 30

[Dave]

Confirming... still annoying me with 2000-09-28-08 build.

Comment 31

[Stephen P. Morse]

Yep, I'm seeing it again.  So either I was imagining things yesterday, or 
something changed and then changed back.  Probably the former.

Comment 32

[cls]

Are there any plans to get this in on the trunk (ie, M18) or are we going to
wait until N6 final (or post 6.0)?

Comment 33

[verah (gone)]

Just wanted to note -- this will be in the release notes for PR3.

Comment 34

[Phil Peterson]

PDT agrees [rtm need info] until code reviews are available. Since we're nervous
about regressions when changing event queue behavior, we'd like a serious
testing plan before giving this [rtm++]

Comment 35

[ratman]

still a problem in mac build 2000100312.  page does not load if user is queried
for initial cookie acceptance, whether or not they are accepted or not, whether
or not the setting is chosen to be remembered.

just a casual update....

Comment 36

[Jeffrey Baker]

This is incredibly annoying.  Shouldn't this be "major" severity?  It is
certianly a loss of function when the page doesn't load.

I'm applying this patch to my tree and will report back soon.

Comment 37

[Doug Turner (:dougt)]

I Checked in a fix for this and this problem should be gone now.  Please verify
with either tomorrow Trunk or NSCP Branch build.

Comment 38

[Decklin Foster]

verifying (current trunk on linux)

Comment 39

[Daniel Veditz [:dveditz]]

I don't think this has been verified on the branch

Comment 40

[Daniel Veditz [:dveditz]]

Sorry for the spammage, no direct way to go from Verified back to resolved. 
This never got rtm++, was it really checked in on the branch?

Comment 41

[Doug Turner (:dougt)]

the real problem was fixed and checked into the branch and trunk.  see 54371 for
the gory details

Comment 42

[Tom Everingham]

verified:
Linux rh6 2000100509  branch