Closed
Bug 321914
Opened 19 years ago
Closed 19 years ago
Crash if a download completes while its properties window is still open
Categories
(Core Graveyard :: Widget: Mac, defect)
Tracking
(Not tracked)
VERIFIED
FIXED
People
(Reporter: starless, Assigned: jaas)
Details
(Keywords: fixed1.8.1, verified1.8.0.2, Whiteboard: [rft-dl])
Attachments
(1 file, 1 obsolete file)
(deleted),
patch
|
mark
:
review+
mconnor
:
superreview+
mconnor
:
approval-branch-1.8.1+
dveditz
:
approval1.8.0.1-
dveditz
:
approval1.8.0.2+
|
Details | Diff | Splinter Review |
User-Agent: Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; it; rv:1.8) Gecko/20051111 Firefox/1.5
Build Identifier: Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; it; rv:1.8) Gecko/20051111 Firefox/1.5
I've got Firefox set up to automatically show the Download Manager when a download starts, and automatically close it when all downloads are completed.
If I start a download, then open the properties window for the currently active download (right click on it in the Download Manager, and select "Properties") and never close it, when the download completes the Download Manager window is automatically closed, but the download properties window is not, and Firefox crashes.
Reproducible: Always
Steps to Reproduce:
1. Set up Firefox to automatically show the Download Manager when a download starts, and automatically close it when all downloads are completed.
2. Be sure that the Download Manager window is closed, then start a download; the Download Manager should open automatically.
3. While the file is still downloading, right-click on its line in the Download Manager window and choose "Properties"; the download properties window will show up.
4. Wait until the download completes. Firefox should crash.
Actual Results:
Firefox crashes.
Expected Results:
Firefox should close the download properties window together with the Download Manager, and continue working as usual.
Talkback crash ID: TB13409971H
Comment 1•19 years ago
|
||
Stack Signature 0xd4069600 8dd40673
Product ID Firefox15
Build ID 2005111116
Trigger Time 2005-12-30 07:30:08.0
Platform MacOSX
Operating System Darwin 8.3.0
Module
URL visited
User Comments A download finished, and the download manager automatically closed (this is the setting I use), while I was still looking at the download's properties window.
Since Last Crash 2166 sec
Total Uptime 3192569 sec
Trigger Reason SIGSEGV: Segmentation Violation: (signal 11)
Source File, Line No. N/A
Stack Trace
0xd4069600
nsMacWindow::WindowEventHandler() [mozilla/widget/src/mac/nsMacWindow.cpp, line 698]
SendEventToEventTargetInternal()
SendEventToEventTargetWithOptions()
HandleWindowEvent()
ToolboxEventDispatcherHandler()
DispatchEventToHandlers()
SendEventToEventTargetInternal()
SendEventToEventTarget()
ToolboxEventDispatcher()
TryEventDispatcher()
GetOrPeekEvent()
GetNextEventMatchingMask()
WNEInternal()
WaitNextEvent()
nsMacMessagePump::GetEvent() nsAppShell::GetNativeEvent() [mozilla/widget/src/mac/nsAppShell.cpp, line 211]
nsXULWindow::ShowModal() [mozilla/xpfe/appshell/src/nsXULWindow.cpp, line 401]
nsWindowWatcher::OpenWindowJS() [mozilla/embedding/components/windowwatcher/src/nsWindowWatcher.cpp, line 603]
nsGlobalWindow::OpenInternal() [mozilla/dom/src/base/nsGlobalWindow.cpp, line 1453]
nsGlobalWindow::OpenDialog() [mozilla/dom/src/base/nsGlobalWindow.cpp, line 4374]
_XPTC_InvokeByIndex()
XPCWrappedNative::CallMethod(XPCCallContext&, XPCWrappedNative::CallMode)() [mozilla/js/src/xpconnect/src/xpcwrappednative.cpp, line 2138]
XPC_WN_CallMethod() [mozilla/js/src/xpconnect/src/xpcwrappednativejsops.cpp, line 1444]
js_Invoke() [mozilla/js/src/jsinterp.c, line 1177]
js_Interpret() [mozilla/js/src/jsinterp.c, line 3526]
js_Invoke() [mozilla/js/src/jsinterp.c, line 1197]
nsXPCWrappedJSClass::CallMethod() [mozilla/js/src/xpconnect/src/xpcwrappedjsclass.cpp, line 1371]
PrepareAndDispatch() [mozilla/xpcom/reflect/xptcall/src/md/unix/xptcstubs_ppc_rhapsody.cpp, line 184]
SharedStub()
nsEventListenerManager::HandleEventSubType() [mozilla/content/events/src/nsEventListenerManager.cpp, line 848]
nsEventListenerManager::HandleEvent() [mozilla/content/events/src/nsEventListenerManager.cpp, line 1766]
nsXULElement::HandleDOMEvent() [mozilla/content/xul/content/src/nsXULElement.cpp, line 2153]
nsXULElement::HandleDOMEvent() [mozilla/content/xul/content/src/nsXULElement.cpp, line 144]
nsXULElement::HandleDOMEvent() [mozilla/content/xul/content/src/nsXULElement.cpp, line 144]
nsXULElement::HandleDOMEvent() [mozilla/content/xul/content/src/nsXULElement.cpp, line 144]
nsEventStateManager::DispatchNewEvent() [mozilla/content/events/src/nsEventStateManager.cpp, line 4566]
nsEventListenerManager::DispatchEvent() [mozilla/content/events/src/nsEventListenerManager.cpp, line 419]
nsDOMEventRTTearoff::DispatchEvent() _XPTC_InvokeByIndex()
XPCWrappedNative::CallMethod(XPCCallContext&, XPCWrappedNative::CallMode)() [mozilla/js/src/xpconnect/src/xpcwrappednative.cpp, line 2138]
XPC_WN_CallMethod() [mozilla/js/src/xpconnect/src/xpcwrappednativejsops.cpp, line 1444]
js_Invoke() [mozilla/js/src/jsinterp.c, line 1177]
js_Interpret() [mozilla/js/src/jsinterp.c, line 3526]
js_Invoke() [mozilla/js/src/jsinterp.c, line 1197]
js_InternalInvoke() [mozilla/js/src/jsinterp.c, line 1275]
JS_CallFunctionValue() [mozilla/js/src/jsapi.c, line 4159]
nsJSContext::CallEventHandler() [mozilla/dom/src/base/nsJSEnvironment.cpp, line 1413]
nsJSEventListener::HandleEvent()
nsEventListenerManager::HandleEventSubType() [mozilla/content/events/src/nsEventListenerManager.cpp, line 848]
nsEventListenerManager::HandleEvent() [mozilla/content/events/src/nsEventListenerManager.cpp, line 1766]
nsXULElement::HandleDOMEvent() [mozilla/content/xul/content/src/nsXULElement.cpp, line 2153]
PresShell::HandleDOMEventWithTarget()
nsMenuFrame::Execute() [mozilla/layout/xul/base/src/nsMenuFrame.cpp, line 848]
nsMenuFrame::HandleEvent() [mozilla/layout/xul/base/src/nsMenuFrame.cpp, line 454]
PresShell::HandleEventInternal()
PresShell::HandleEvent()
nsViewManager::HandleEvent() [mozilla/view/src/nsViewManager.cpp, line 61]
nsViewManager::DispatchEvent() [mozilla/view/src/nsViewManager.cpp, line 2246]
HandleEvent() [mozilla/view/src/nsView.cpp, line 175]
nsWindow::DispatchEvent() [mozilla/widget/src/mac/nsWindow.cpp, line 1809]
nsWindow::DispatchWindowEvent() [mozilla/widget/src/mac/nsWindow.cpp, line 1825]
nsWindow::DispatchMouseEvent() [mozilla/widget/src/mac/nsWindow.cpp, line 1851]
nsMacEventHandler::HandleMouseUpEvent()
nsMacEventHandler::HandleOSEvent()
nsMacWindow::DispatchEvent() [mozilla/widget/src/mac/nsMacWindow.cpp, line 1730]
nsMacMessagePump::DispatchOSEventToRaptor()
nsMacMessagePump::DoMouseUp()
nsMacMessagePump::DispatchEvent()
nsMacMessagePump::DoMessagePump()
nsAppShell::Run() [mozilla/widget/src/mac/nsAppShell.cpp, line 114]
nsAppStartup::Run()
XRE_main() [mozilla/toolkit/xre/nsAppRunner.cpp, line 2315]
_start()
start()
Assignee: nobody → joshmoz
Component: Download Manager → Widget: Mac
Product: Firefox → Core
QA Contact: download.manager → mac
Version: unspecified → 1.8 Branch
This patch really fixes another problem. When a user starts interacting with the downloads window, it should no longer autoclose. A side effect of this fix is that this bug is done away with.
We should probably open a bug on the core cause of this, which is probably a widgets issue.
Attachment #207214 -
Flags: superreview?
Attachment #207214 -
Flags: review?(mark)
Attachment #207214 -
Flags: superreview? → superreview?(mconnor)
Comment 3•19 years ago
|
||
Comment on attachment 207214 [details] [diff] [review]
fix v1.0
You need to restore gCanAutoClose when the properties sheet/dialog closes.
Attachment #207214 -
Flags: superreview?(mconnor)
Attachment #207214 -
Flags: review?(mark)
Attachment #207214 -
Flags: review-
I don't want to restore the flag, but I did set the wrong flag. New patch coming up.
Comment 5•19 years ago
|
||
Comment on attachment 207214 [details] [diff] [review]
fix v1.0
OK, I thought that making the window not close after opening a download's properties was an unintended result of this patch, but since it's intended and desired, the behavior is fine with me.
Attachment #207214 -
Flags: review-
This is a better way to do it. Right now, the dl manager does not take into account whether or not a user has interacted with the window in a significant way. So, I added a variable to keep track of user interaction and if that variable is true, then the dl manager does not auto-close. This fixes this particular bug at a higher level, and improves the UI a bit.
There are tougher UI decisions to make about what should be considered significant user interaction, but this is not the place for that discussion and I don't want to include anything further in my patch on that subject. I think we can all agree that opening the properties dialog is significant enough interaction to stop auto-closing.
Attachment #207214 -
Attachment is obsolete: true
Attachment #207594 -
Flags: review?(mark)
Note: my "fix v1.0" does almost exactly the same thing as "fix v2.0." The difference is that |gCanAutoClose| is used for xpinstall stuff, and is set in 2 places. In theory one of those assignments could overwrite a |true| value that got assigned as a result of user interaction. I don't know if that is likely to happen, but "fix v2.0" is a much more clear way of handling the situation, and avoids the theoretical problems.
Updated•19 years ago
|
Attachment #207594 -
Flags: review?(mark) → review+
Attachment #207594 -
Flags: superreview?(mconnor)
Updated•19 years ago
|
Flags: blocking1.8.0.1? → blocking1.8.0.1-
Updated•19 years ago
|
Attachment #207594 -
Flags: superreview?(mconnor) → superreview+
Attachment #207594 -
Flags: approval1.8.1?
Attachment #207594 -
Flags: approval1.8.0.1?
Status: ASSIGNED → RESOLVED
Closed: 19 years ago
Resolution: --- → FIXED
Comment 9•19 years ago
|
||
beltzner: this changes UI behavior to sidestep the crash. Is this behavior change OK?
This has been checked into the trunk, we'd like to see more baking time.
Updated•19 years ago
|
Attachment #207594 -
Flags: approval1.8.0.2?
Attachment #207594 -
Flags: approval1.8.0.1?
Attachment #207594 -
Flags: approval1.8.0.1-
Comment 10•19 years ago
|
||
(In reply to comment #2)
> This patch really fixes another problem. When a user starts interacting with
> the downloads window, it should no longer autoclose. A side effect of this fix
> is that this bug is done away with.
Really? I'm not sure I agree with that. The option clearly states "when downloads are finished", not "if I don't touch it". I think the reporter is actually correct in stating that the expected behaviour would be that any attached sheets would also close.
> We should probably open a bug on the core cause of this, which is probably a
> widgets issue.
So is this the only way to work around it until that time?
Assignee | ||
Comment 11•19 years ago
|
||
Beltzner - the idea behind autoclose is to keep the download manager out of your way when you're just watching something complete. But if you're doing more than that, actually interacting with it, automatically closing can be disruptive. The author of the autoclose code even noted in the code that he/she thought we may want to do this. I don't think being absolute about the wording of the option here is in the user's best interest.
If you saw the download manager and clicked on the properties dialog, were reading that or editing something in it, would you really want the download manager to close (possibly with your properties window) from under you?
Ben? Mconnor?
Comment 12•19 years ago
|
||
(In reply to comment #9)
> beltzner: this changes UI behavior to sidestep the crash. Is this behavior
> change OK?
Yeah, definitely OK for a stability release. ui-approval = me for 1.8.0.x release.
(In reply to comment #11)
> thought we may want to do this. I don't think being absolute about the wording
> of the option here is in the user's best interest.
That's not really what I was trying to do. I was trying to interpret what a user might be expecting based on that option being clicked.
> If you saw the download manager and clicked on the properties dialog, were
> reading that or editing something in it, would you really want the download
> manager to close (possibly with your properties window) from under you?
So, you can't edit in that properties dialog, you can only inspect & copy. And on Mac, that properties window is an attached sheet (is that what's causing the problem in the first place?). At that point, if I'd clicked "get rid of this dialog when things are done downloading", then yes, I'd expect it to go away as it's a read only display which has an inspector for more details.
Comment 13•19 years ago
|
||
If you're looking at the properties of Download A while Download B is chugging away, I don't think its at all expected behaviour for that window to close while you're looking at it just because Download B is finished. The primary use-case for autoclose is "User wants to see the download only while it is in progress" but as soon as you're interacting with completed downloads, you're shifting into a different use-case (user wants to get information about previous downloads) and auto-close shouldn't apply, just as it doesn't when you manually open the download manager.
Comment 14•19 years ago
|
||
I guess I'm thinking of the properies window (which is an edge, edge case - c'mon, how many people really look at that thing?) as an "inspector". If we'd built the window like this:
.---------------------------------------------.
|*someFile* _Open_ |
| _Remove_ |
| _Details_|
|---------------------------------------------|
| someOtherFile _Open_ |
| _Remove_ |
| from: http://www.foo.com/ |
| to : ~/fooplace |
| completed on Feb 14th, 2009 _Hide Details_|
|---------------------------------------------|
| |
: :
then I don't think we'd be having this discussion. From a user task basis, that's pretty much all the properties window really lets you do.
Updated•19 years ago
|
Attachment #207594 -
Flags: approval1.8.1? → branch-1.8.1?(mconnor)
Updated•19 years ago
|
Attachment #207594 -
Flags: branch-1.8.1?(mconnor) → branch-1.8.1+
Assignee | ||
Comment 15•19 years ago
|
||
landed on MOZILLA_1_8_BRANCH
Updated•19 years ago
|
Keywords: fixed1.8.1
Comment 16•19 years ago
|
||
Comment on attachment 207594 [details] [diff] [review]
fix v2.0
approved for 180 branch, a=dveditz for drivers
Attachment #207594 -
Flags: approval1.8.0.2? → approval1.8.0.2+
Updated•19 years ago
|
Flags: blocking1.8.0.2+
Comment 18•19 years ago
|
||
Is this a 10.2-only bug, or can we verify it on 10.3/10.4?
Whiteboard: [rft-dl]
Assignee | ||
Comment 19•19 years ago
|
||
This happens on all versions of Mac OS X.
Comment 20•19 years ago
|
||
verified fixed and behaving as in comment #6 (not closing upon completion because the properties dialog was open)
Mac 2006-02-27-05-mozilla1.8.0
Status: RESOLVED → VERIFIED
Keywords: fixed1.8.0.2 → verified1.8.0.2
You need to log in
before you can comment on or make changes to this bug.
Description
•