224449 - Clicking Cancel doesn't close download window

Status: VERIFIED FIXED

(Core Graveyard :: File Handling, defect)

Description

[Frederic Bezies]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.6a) Gecko/20031102 Firebird/0.7+ (Gcc 3.3.1 i686 optimized - MozJF)
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.6a) Gecko/20031102 Firebird/0.7+ (Gcc 3.3.1 i686 optimized - MozJF)

All is in the title :)

Reproducible: Always

Steps to Reproduce:
1.Build firebird from a up-to-date CVS trunk
2.launch a download
3.try to resume / cancel it

Actual Results:  
Nothing happens. I have to close the window using "x" button.

Expected Results:  
cancel, pause or resume download.


I got this in JS console :

Error: uncaught exception: [Exception... "Component returned failure code:
0x8000ffff (NS_ERROR_UNEXPECTED) [nsIRequest.resume]"  nsresult: "0x8000ffff
(NS_ERROR_UNEXPECTED)"  location: "JS frame ::
file:///C:/Program%20Files/MozillaFirebird/components/nsProgressDialog.js ::
anonymous :: line 695"  data: no]

Here is the .mozconfig I used to build this firebird (I use gcc-win32 for better
speed and optimization)

# ------------------
# GCC BUILD SETTINGS:
# ------------------
CC=gcc
CXX=g++
CPP=cpp
AS=as
LD=ld
LDFLAGS=-mwindows

export MOZ_PHOENIX=1
mk_add_options MOZ_PHOENIX=1

ac_add_options --disable-ldap
ac_add_options --disable-mailnews
ac_add_options
--enable-extensions=cookie,xml-rpc,xmlextras,p3p,pref,transformiix,universalchardet,typeaheadfind,webservices
ac_add_options --enable-crypto
ac_add_options --disable-composer
ac_add_options --disable-profilesharing
ac_add_options --disable-installer

mk_add_options MOZ_OBJDIR=@TOPSRCDIR@/../fbbuild

ac_add_options --disable-accessibility
ac_add_options --disable-debug
ac_add_options --disable-tests

ac_add_options --enable-strip
ac_add_options --enable-strip-libs

ac_add_options --enable-optimize="-Os -march=i686"

Comment 1

[Frederic Bezies]

Is this bug related - in some way ? - to bugfix for bug 223099 ?

Comment 2

[Frederic Bezies]

Well, I don't understand what is happening. Pause / resume works again.

But cancel is still broken, giving the JS console error. Modifying summary.

It should be consider as a regression, or not ?

Comment 3

[Ali Ebrahim]

I can confirm on: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.6a)
Gecko/20031102 Firebird/0.7+ (aebrahim). CVS pull at midnight PST today.

Comment 4

[Jesse Ruderman]

I see this bug when downloading http://members.cox.net/impunity/endofworld.swf with
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.6a) Gecko/20031031
Firebird/0.7+ (Gcc 3.3.1 i686 optimized - MozJF)

The download stops, but the window doesn't close.  I get the error in the JS
console mentioned in comment 0.

Comment 5

[Frederic Bezies]

Your comment confirm what I thought. Bugfix for bug 223099, added on 30th
october should (must ?) be guilty for this regression.

Comment 6

[Wayne Woods]

I'm having the same problem. The first time I hit cancel, the download stops,
but the window stays open. But I can usually get the download window to close by
hitting cancel a second time. Win XP 20031102 Firebird/0.7+

Comment 7

[Darin Fisher]

neil: can you look into this?  thx!

Comment 8

[Christian :Biesinger (don't email me, ping me on IRC)]

hm, I get the js error in mozilla too...

neil: previously, the code only set this.mRequest if the previous request was
null. now it always sets it. could that be the reason for this bug?

I also observe that clicking Cancel doesn't close the dialog, also in mozilla.
(but I can click "Close" after that)

hence, -> browser/file handling

Comment 9

[[e-mail]]

OK, so I tried everything I thought of.
I put in dump()s and all the requests are identical objects.
I backrevved to 1.32 and still had the issue.
I tried save link vs clicking the link.
I then upped to 1.33 again and the issue went away.

So, I can't help, because I can't reproduce the bug any more.

BTW the ftp.mozilla.org mirrors have different images, very confusing :-/

Comment 10

[Boris Zbarsky [:bzbarsky]]

The problem is that the onCancel handler in nsProgressDialog.js does

426          if ( !this.completed ) {
...
434              this.paused = false;
435          }

and since mPaused is initialized to _null_ instead of false in the constructor,
the "if ( this.paused != pausing )" test in setPaused tests true and we try to
call resume() on a channel we never suspended.  nsHttpChannel throws if it's
already not suspended and you resume it, which is the error we get here (see
http://lxr.mozilla.org/seamonkey/source/netwerk/base/src/nsInputStreamPump.cpp#174
for the actual place where the NS_ERROR_UNEXPECTED is thrown).

Initing mPaused to false instead of null should fix this, methinks.

Comment 11

[[e-mail]]

Attachment: bz's fix

Why do we have all these getters shadowing properties anyway?

Comment 12

[Christian :Biesinger (don't email me, ping me on IRC)]

ahh. so the patch that I really suspected (i.e. "nshttpchannel always returns
ns_ok") was the one that caused this.
I didn't mention it in this bug because I could reproduce with FTP urls as well

Comment 13

[Darin Fisher]

Comment on attachment 134770 [details] [diff] [review]
bz's fix

sr=darin

biesi: isn't it fun how folks depend on undocumented implementation behavior of
nsIRequest :-/

in this case, i imagine it's just coincidence.

Comment 14

[[e-mail]]

Fix checked in.

Comment 15

[Boris Zbarsky [:bzbarsky]]

I hope someone tested that patch, since my comment was based on code inspection
only...

Comment 16

[Frederic Bezies]

Well, I will test this patch by building a new bird. I hope that in something
like one hour I can add a comment and said : "Ok, bug is dead" :)

Comment 17

[[e-mail]]

Yes, in fact it even explained why I had so much difficulty reproducing the bug
- pausing the download makes the bug go away, of course :-[

Comment 18

[Frederic Bezies]

Bug is now dead ! Yeah :)

Thanks bz for fixing it !

Comment 19

[Bill Mason]

v