Cancel download button should not be click through

RESOLVED FIXED in Camino1.5

Status

P3
enhancement
RESOLVED FIXED
16 years ago
14 years ago

People

(Reporter: stf, Assigned: nick.kreeger)

Tracking

unspecified
Camino1.5
PowerPC
macOS

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Reporter)

Description

16 years ago
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

Comment 1

16 years ago
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

Comment 2

15 years ago
taking
Assignee: sdagley → josha

Comment 3

15 years ago
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.

Comment 4

15 years ago
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.

Comment 5

15 years ago
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.

Comment 6

14 years ago
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?

Updated

14 years ago
Priority: -- → P3
Target Milestone: --- → Camino1.1
(Assignee)

Comment 7

14 years ago
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.
(Assignee)

Comment 9

14 years ago
Will make the pause/resume button the same way..
(Assignee)

Comment 10

14 years ago
Created attachment 191439 [details] [diff] [review]
Turns off click through for all buttons 

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.
(Assignee)

Comment 14

14 years ago
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.
(Assignee)

Comment 17

14 years ago
Created attachment 191481 [details] [diff] [review]
Allows open to be click-through

Patch now allows click-through
Attachment #191481 - Flags: review?(pinkerton)
(Assignee)

Updated

14 years ago
Attachment #191439 - Attachment is obsolete: true

Comment 18

14 years ago
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).
(Assignee)

Comment 20

14 years ago
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.

Updated

14 years ago
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
Last Resolved: 14 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.