Closed Bug 192223 Opened 22 years ago Closed 19 years ago

Cancel download button should not be click through

Categories

(Camino Graveyard :: Downloading, enhancement, P3)

Product:
Camino Graveyard
Component:
Downloading
Platform:
PowerPC
macOS
Type:
enhancement

Tracking

(Not tracked)

Status:
RESOLVED FIXED
Milestone:
Camino1.5

People

(Reporter: stf, Assigned: nick.kreeger)

Details

Attachments

(1 file, 1 obsolete file)

Allows open to be click-through
3.65 KB, patch
mikepinkerton
: review+
Details | Diff | Splinter Review 
If you click on the Cancel download button (x) in the Download window when that
window is in (local/global) background the download is cancelled.

Until the Cancel button is enhanced to became a Reload when a download has been
interrupted or completed, the click through is really nasty.
Build 2003020607
Confirming this on the latest nightly (2003081002). If you're on a 56k modem
cancelling the download of the latest nightly just because the button was in the
background is definitely a "potentially harmful" action, see the HIG chapter 5.
Status: UNCONFIRMED → NEW
Ever confirmed: true
taking
Assignee: sdagley → josha
Can't do much about this now since our cancel button is an NSToolbarItem. We
would have to subclass it and somehow replace its internal view with a subclass
of NSView. I'm not gonna go there. I don't think anybody else wants to either.
I have a solution to this bug, but I'm not sure its wanted. NSToolbar items are
clickthrough by default - do we really want the download manager to behave
differently? This comes down to safety vs. consistency.

Simple solution - do nothing in the action method if the window is in the
background. Its a very small patch.

As the original description suggests, the real fix is to make downloads
continuable. I really hate buttons that don't work in the background when most
do (such as the 10.3 Finder volume eject buttons).

On the other hand, we could do this as a temporary measure then remove it again
after downloads are restartable.
Hmm... it must be possible. Mail.app has a non-click through delete button on
its toolbar, but its other toolbar buttons are click through. Could intercepting
"become foreground window", "no longer foreground window" messages and
enabling/disabling the button be another approach?
Priority: -- → P3
Target Milestone: --- → Camino1.1
Looks like mail.app un-enables the delete button if the window is not the
current selected window, maybe an easy patch, I will investigate
(In reply to comment #7)
> Looks like mail.app un-enables the delete button if the window is not the
> current selected window, maybe an easy patch, I will investigate

Check out the NSToolbarItemValidation protocol.
Will make the pause/resume button the same way..
Turns off pause/resume, open, remove, cancel.. checks for keywindow
Attachment #191439 - Flags: review?(pinkerton)
I want to echo Stuart's statement in comment 5; now that we (presumably) have
pause/resume working, there's little need to disable cancel (unless cancel can't
yet be resumed?), and no need to disable any of the other buttons. 

It's *very* handy to be able to open/show/clean up items without having to bring
the downloads manager to the foreground, and comment 10 would be a real step
backwards.  We should fix bugs, not disable functionality to work around bugs.
(In reply to comment #11)
> I want to echo Stuart's statement in comment 5; now that we (presumably) have
> pause/resume working, there's little need to disable cancel (unless cancel can't
> yet be resumed?), and no need to disable any of the other buttons. 
> 
> It's *very* handy to be able to open/show/clean up items without having to bring
> the downloads manager to the foreground, and comment 10 would be a real step
> backwards.  We should fix bugs, not disable functionality to work around bugs.

For me it's all about loss of data. Say we don't disable click through for any
of the buttons. I, as a 56k user (hypothetically), accidentally hit cancel, then
clean up. I've just lost my download and download location. I can't resume that.
Also, if I remember correctly, cancelling currently deletes the partial file.
That may be wrong, so please correct me. 

We can potentially interrupt a user's "flow" if they accidentally hit any button
except "reveal". They can lose data from a cancel, a remove/clean up (in case
they wanted to remember which downloads they'd done recently), and a pause (in
case the server doesn't correctly support pause/resume). In addition, "open" can
be potentially dangerous/unexpected if a user doesn't realize they're opening
the file or even that they downloaded it (our download manager could be in the
background when they click a link).

Does that make sense? We should have more discussion about this before
implementing, that's for sure.
I don't think that *all* of the buttons need to be disabled when the window is
not key, but the cancel button definitely should be.
To make it more clear, this is what the attached patch does:

Disables these buttons for click through:
1. Remove
2. Cancel
3. Pause/Resume
4. Open

These buttons are still enabled for click through:
1. Clean Up
2. Show

cancel is still "dangerous" in that it's not easily resumable (that's another
bug that needs work). so i agree with kreeger's summary in the previous comment.
i'm ok with moving "open" to the list of allowed clickthroughs. i agree it's
useful and we can only do so much to prevent users from downloading and opening
things that are bad.
Patch now allows click-through
Attachment #191481 - Flags: review?(pinkerton)
Attachment #191439 - Attachment is obsolete: true
So this patch actually disables the buttons when the window is not frontmost?
That's a bit weird, and gives the wrong feedback. I think a custom view with an
-acceptsFirstMouse: override would be preferrable. Is it really hard?
Probably not, but this is what pretty much every other app with a delete button
in the toolbar does (e.g., Mail).
I agree with Wevah, most mac apps have this so someone doesnt accidently press
delete.. An alternative option is to check to see if we are in the key window in
the actual selector that the button executes |ie. -(IBAction) cancel:(id)sender|
and allow the button to actually execute if its the key window, that would get
around having to screw with writing a custom view, and will probly use much less
code just for this simple feature

IIRC, though, the window becomes key/main before the action is fired.

console app disables the "clear" button when the window is in the bg. bringing
it to the fg enables the button.
Assignee: joshmoz → nick.kreeger
Comment on attachment 191481 [details] [diff] [review]
Allows open to be click-through

r=pink.
Attachment #191481 - Flags: review?(pinkerton) → review+
landed.
Status: NEW → RESOLVED
Closed: 19 years ago
Resolution: --- → FIXED
Attachment #191439 - Flags: review?(pinkerton)
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: