Triggering w/missing .xpi results in progress dlog hanging

VERIFIED FIXED in mozilla0.9

Status

Core Graveyard
Installer: XPInstall Engine
P3
normal
VERIFIED FIXED
18 years ago
2 years ago

People

(Reporter: David Epstein, Assigned: dbragg)

Tracking

Trunk
mozilla0.9
x86
Windows NT

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [xpibug][rtm-], URL)

Attachments

(2 attachments)

(Reporter)

Description

18 years ago
builds 2000-09-01-10-M18. cross platform (on Linux, it didn't do this the first 
2 times).
1. Go to http://jimbob/trigger3.html
2. Press Trigger for Trigger URL field. Don't enter a .xpi.
3. When confirm dialog appears, press OK.
Result: On NT, Progress bar starts running, but keeps going and going ... for 
1/2 hour last time I checked. On Mac and Linux, the progress bar didn't work.
Expected: Check for .xpi when using InstallTrigger method. If not, then timeout 
should work.
Consider for beta 3, but probably not since webpage authors can work around 
this.
Keywords: nsbeta3
workaroundable on the webdesigner's side, nsbeta3-
Whiteboard: [nsbeta3-]
(Reporter)

Comment 3

17 years ago
Nominating for rtm. If a wrong URL is triggered, it hangs the XP Confirm dialog. 
The browser can be used, but it will operate very slowly. For the Mac, see 55567 
for what happens when this is followd by a force-quit.
Keywords: rtm
Too much an edge-case for RTM: developers can debug their sites and work around
the problem.
Whiteboard: [nsbeta3-] → [nsbeta3-][rtm-]
This bug could be used maliciously, plus could be accidental site damage
(machine down?). Not in the same class as debuggable script errors that result
in a crash, this is more danger to users.
Keywords: nsbeta3 → hang, nsbeta1
Whiteboard: [nsbeta3-][rtm-] → [rtm-]
Over to Don. Necko probably returns some status code in this case that we're 
not interpreting correctly.
Assignee: dveditz → dbragg
Whiteboard: [rtm-] → [xpibug][rtm-]
Moz 0.9 tasks
Target Milestone: --- → mozilla0.9

Updated

17 years ago
Keywords: hang, nsbeta1, rtm → nsbeta1+
(Assignee)

Comment 8

17 years ago
This seems to be working now.  Marking WORKSFORME.
Status: NEW → RESOLVED
Last Resolved: 17 years ago
Resolution: --- → WORKSFORME
If I enter a completely blank URL we seem to figure out the -228 error, but if 
we enter a valid host with an invalid file we make the connection and then spin 
up to 100% CPU. There's still a problem here.

try
javascript:InstallTrigger.install({"test":"ftp://ftp.netscape.com/"},function(u
rl,status){url+' '+status);})
Status: RESOLVED → REOPENED
Resolution: WORKSFORME → ---
(Assignee)

Comment 10

17 years ago
Actually this bug has "migrated".  That's why I marked it WORKSFORME.  It's not
the Confirm dialog that's hanging.
The problem is that our OnStopRequest callback gives us the error but we
continue to call DownloadNext.  (All this is in nsXPInstallManager.cpp)  It
looks like we clean up ok but obviously something is still happening.

Investigating.
(Assignee)

Comment 11

17 years ago
Created attachment 25751 [details] [diff] [review]
Patch to prevent multiple shutdowns
(Assignee)

Comment 12

17 years ago
Posted patch.  Bracketing the Shutdown code and setting mDlg to nsnull after
it's been closed, otherwise we try to close it everytime since mDlg->Close()
doesn't reset the object to null.

Ran 3 tests back-to-back with a bogus ftp path (valid host but invalid path).
No crashes and no dialog hangs.  Also tried several standard QA tests.
The summary was wrong, if you look at the steps it was always the progress 
dialog. fixing summary and shortening so it isn't truncated on bug listings
Summary: Triggering w/o specified .xpi results in confirm dlog hanging → Triggering w/missing .xpi results in progress dlog hanging

Comment 14

17 years ago
r=ssu

Comment 15

17 years ago
sr=mscott
I had cut and paste problems in the test URL, left out "alert("--and the URL 
field is just whacked entirely.

javascript:InstallTrigger.install({"test":"ftp://ftp.netscape.com/"},function(u
rl,status){alert(url+' '+status);})

The patch shouldn't use mDlg as the flag to indicate shutdownness. mDlg could 
be null in the future for other reasons and we'd still have to clean up the 
files.
(Assignee)

Comment 17

17 years ago
Created attachment 25887 [details] [diff] [review]
New patch using specific shutdown flag instead of mDlg.
Looks good, r=dveditz

Comment 19

17 years ago
sr=mscott
(Assignee)

Comment 20

17 years ago
Fix checked in yesterday (2/22/01). Should be in today's verification builds.
Status: REOPENED → RESOLVED
Last Resolved: 17 years ago17 years ago
Resolution: --- → FIXED

Comment 21

17 years ago
Build: 2001-03-01-11-Mtrunk(WIN), 2001-03-01-09-trunk(MAC), 
2001-03-01-08-Mtrunk(LINUX)

Looks good now.  However, we are still experiencing a crash in Linux (Bug 
70635).

Marking this one Verified.
Status: RESOLVED → VERIFIED
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.