Closed
Bug 228699
Opened 21 years ago
Closed 20 years ago
Download Manager should close on CTRL+Y / ESC
Categories
(Toolkit :: Downloads API, enhancement, P3)
Toolkit
Downloads API
Tracking
()
RESOLVED
FIXED
mozilla1.7.4
People
(Reporter: athakur, Assigned: steffen.wilberg)
References
()
Details
(Keywords: fixed-aviary1.0, Whiteboard: [have patch])
Attachments
(1 file, 7 obsolete files)
2.15 KB,
patch
|
bugs
:
review+
bugs
:
approval-aviary+
|
Details | Diff | Splinter Review |
User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.6b) Gecko/20031216 Firebird/0.7+
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.6b) Gecko/20031216 Firebird/0.7+
All of the sidebar windows can be closed using the same button that opens them
(for example, pressing "CTRL-B" opens the bookmarks window, hitting "CTRL-B"
closes it again). The new download manager window does not follow the same
behavior.
I think the download manager should follow the same behaviour both for
consistency and ease of use.
Reproducible: Always
Steps to Reproduce:
1. Hit CTRL-E to bring up the Download Manager
2. Hit CTRL-E again
Actual Results:
Nothing happens
Expected Results:
Download Manager window should close and focus return to the browser window from
which the Download Manager was initially called
Comment 1•21 years ago
|
||
focus returning to the browser window it was spawned from is bad if you're in a
second or third window, because then you need to switch back to where you're
hitting Ctrl-E
-> NEW
Status: UNCONFIRMED → NEW
Ever confirmed: true
OS: Windows 2000 → All
QA Contact: mconnor
Hardware: PC → All
I'm sure the framework is present in the browser to remember the state at which
the browser is layered and basic window managment. So that if you press CTRL+E
and you had no Firebird Windows maximized it would just close the download
manager and leave the screen, which would cause the Firbird to lose focus.
Besides closing download manager with the same starting key combination is the
most intuitive response.
Comment 3•21 years ago
|
||
target.
Status: NEW → ASSIGNED
Priority: -- → P3
Target Milestone: --- → Firebird0.10
Comment 4•21 years ago
|
||
*** Bug 236398 has been marked as a duplicate of this bug. ***
Comment 5•21 years ago
|
||
If we're doing this for the Download Manager window, does it make sense to have
Ctrl+I close to Page Info window, and just implement this system of
same-shortcut-to-close-window for this class of windows?
Not necessarily. The reason for the Ctrl-E toggle for the DM is the precedent
that was set when the component was a sidebar.
I think Ali's suggestion is not a bad idea. Perhaps pressing a dialog's hotkey
should dismiss the dialog if it has focus; otherwise it could open or bring the
dialog to the front (like Winamp 5's "Bring to front/hide Winamp" global hotkey,
if anyone uses that).
Minor add: when making CTRL-E a toggle the relating toolbar button for the
downloads window should act the same way (toggling and showing a pressed state
and a released state)
Comment 9•21 years ago
|
||
See bug 233440 - Change DL Manager Keybinding.
Summary: Download Manager should close on CTRL-E → Download Manager should close on CTRL+Y
Assignee | ||
Comment 10•21 years ago
|
||
*** Bug 242882 has been marked as a duplicate of this bug. ***
Assignee | ||
Updated•21 years ago
|
Summary: Download Manager should close on CTRL+Y → Download Manager should close on CTRL+Y / toolbar button
Updated•21 years ago
|
Flags: blocking1.0+
Assignee | ||
Comment 11•21 years ago
|
||
Simply adds a second close keybinding to the downloads manager (in addition to
ctrl-w). The effect on hitting ctrl-y is this:
If the downloads manager is not open, it is opened and gets focus (no change
here).
If the downloads manager is already open, but doesn't have focus, it is brought
up front and becomes focus (no change here as well).
If the downloads manager has focus, it is closed, just like hitting ctrl-w or
clicking on the close window widget.
So you can hit ctrl-y to have a short look in the manager, and hit ctrl-y again
to get rid of it.
Assignee: bugs → steffen.wilberg
Assignee | ||
Comment 12•21 years ago
|
||
Comment on attachment 150398 [details] [diff] [review]
patch for to make ctrl-y close it
Doesn't include the toolbarbutton yet.
Attachment #150398 -
Attachment description: patch → patch for to make ctrl-y close it
Updated•21 years ago
|
Priority: P3 → P4
Comment 13•21 years ago
|
||
*** Bug 248135 has been marked as a duplicate of this bug. ***
Comment 14•21 years ago
|
||
I request to RESOLVE this bug via changing CTRL+Y back to CTRL+E. It is
important USABILITY ISSUE!
You cannot close DM by one hand pressing CTRL+Y. Try it.
But you can to close it easily via CTRL+E.
Comment 15•21 years ago
|
||
Typing in all caps is annoying. So is going offtopic and ranting about things
outside of the context of a specific bug.
Comment 16•21 years ago
|
||
(In reply to comment #15)
> Typing in all caps is annoying. So is going offtopic and ranting about things
> outside of the context of a specific bug.
And you are the greatest man on the earth.
Comment 17•21 years ago
|
||
(In reply to comment #16)
> And you are the greatest man on the earth.
its always good to know I still have fans.
<backontopic/>
Steffen, any update on this? I'd like to see this sooner rather than later :)
Assignee | ||
Comment 18•21 years ago
|
||
Comment on attachment 150398 [details] [diff] [review]
patch for to make ctrl-y close it
Let's start with this one, before bsmedberg's large patch-bitrot (toolkit
locale move) takes place.
Attachment #150398 -
Flags: review?(mconnor)
Assignee | ||
Comment 19•21 years ago
|
||
The toolbarbutton is much harder to fix. It's possible to detect whether the
download manager is open or not, by getMostRecentWindow(Download:Manager). So I
could easily make the button close the download manager if it's open.
But is that what we want? If the downloads manager is already open but hidden
behind the main window, it should IMHO not be closed, but brought up front and
become focus, like ctrl-y does with my patch applied. On the other hand, it
wouldn't be that bad if the toolbarbutton toggled open and close, no matter
where the download manager is.
There are some z-order functions in nsIWindowMediator. So we could possibly
detect whether the download manager is on top, and only close it in that case.
The problem is that in the moment you click on the toolbarbutton, the main
window is up front. But how can we know whether the download manager was brought
behind the main window through clicking on the toolbarbutton, or whether it was
there all the time? I don't know if "onmousedown" helps here.
It's possible to make the download manager always stay on top (alwaysRaised
flag), but that's pretty annoying.
Comment 20•21 years ago
|
||
The answer depends on whether we're going to have a pressed state for the
downloads toolbar button.
If we do:
Ctrl-Y/toolbar button should act as (effectively) a show/hide toggle.
If we don't:
clicking the toolbar button should be a single action (open/bring to front).
Ctrl-Y can still be used to close it if the Download Manager window has focus,
but if not we should bring it into focus.
I like the toggle concept better, we just need to implement the pressed state
for it to be coherent.
QA Contact: mconnor → bmo
Assignee | ||
Comment 21•21 years ago
|
||
Here's the toggle. Click: open. Second click: close. Ctrl-y does the same. No
matter if the download manager is on top or behind, has focus or not, it is
closed if it's already open, and is opened if it's not open yet.
The toolbarbutton now has a pressed state, which is the same for all windows. I
need an observer to inform the other windows.
The only issue I noticed is that the button doesn't get the pressed state when
adding it to the toolbar and the download manager is open.
Assignee | ||
Updated•21 years ago
|
Attachment #150398 -
Attachment is obsolete: true
Assignee | ||
Updated•21 years ago
|
Attachment #150398 -
Flags: review?(mconnor)
Assignee | ||
Comment 22•21 years ago
|
||
This fixes the issue I mentioned before by setting the "checked" attribute on
the always-present "Tools:Downloads" command, instead of the downloads-button
itself. Renamed the observer from "Tools:Downloads", which could be confusing,
to DM-open-close. Moved the notifyObserver() calls to downloads.js since I need
the close observer there anyway.
Assignee | ||
Updated•21 years ago
|
Attachment #152652 -
Attachment is obsolete: true
Assignee | ||
Comment 23•21 years ago
|
||
removed a now unnecessary if.
Attachment #152691 -
Attachment is obsolete: true
Assignee | ||
Comment 24•21 years ago
|
||
I really hate removing traces of other changes by hand.
Attachment #152694 -
Attachment is obsolete: true
Assignee | ||
Comment 25•21 years ago
|
||
Comment on attachment 152695 [details] [diff] [review]
toogle v1.0.2
Here you are!
Attachment #152695 -
Flags: review?(mconnor)
Assignee | ||
Comment 26•21 years ago
|
||
Requesting blocking RC1 due to localization impact, and because I wouldn't mind
some public testing.
Flags: blocking-aviary1.0RC1?
Comment 27•21 years ago
|
||
If it were up to me, I'd mark this won't fix. See bug 176177 comment 33 and bug
176177 comment 35. But it's not up to me it's up to Ben, so I'll be quiet.
Comment 28•21 years ago
|
||
I think the current behavior of the toolbar button and ctrl-y is fine. If we
want to be able to close the window, why no just make the esc key or crtl-e
close it (see bug 233529). Would that work?
Assignee | ||
Comment 29•21 years ago
|
||
We can of course just make Ctrl-Y close it, without changing the toolbarbutton.
See comment 11.
Assignee | ||
Comment 30•21 years ago
|
||
Comment on attachment 152695 [details] [diff] [review]
toogle v1.0.2
>Index: mozilla/browser/base/content/browser.js
>+ init: function()
...
>+ // copy the state of the download manager button from the opening window, if any
>+ if (window.opener && window.opener.document.getElementById("Tools:Downloads").getAttribute("checked") == "true")
>+ document.getElementById("Tools:Downloads").setAttribute("checked", true);
This has to be replaced by:
> // set the state of the download manager button
> if (this._getDM())
> document.getElementById("Tools:Downloads").setAttribute("checked", "true");
because not every opener has "Tools:Downlaods", for example the extension/theme
manager when clicking on the u.m.o link.
Assignee | ||
Comment 31•21 years ago
|
||
*** Bug 233529 has been marked as a duplicate of this bug. ***
Assignee | ||
Comment 32•21 years ago
|
||
From the dupe: The download manager should be closed on ESC as well.
Summary: Download Manager should close on CTRL+Y / toolbar button → Download Manager should close on CTRL+Y / ESC / toolbar button
Assignee | ||
Comment 33•21 years ago
|
||
- makes the changes I mentioned in comment 30.
- makes the download manager close on ESC (like Bookmarks Manager, Page Info,
Options etc.)
Attachment #152695 -
Attachment is obsolete: true
Assignee | ||
Updated•21 years ago
|
Attachment #152695 -
Flags: review?(mconnor)
Assignee | ||
Updated•21 years ago
|
Attachment #153072 -
Flags: review?(mconnor)
Assignee | ||
Updated•21 years ago
|
Attachment #153072 -
Flags: review?(mconnor)
Assignee | ||
Updated•21 years ago
|
Attachment #153186 -
Flags: review?(mconnor)
Updated•21 years ago
|
Whiteboard: http://testrunner.mozilla.org/test.cgi?run_id=244#892
Comment 35•21 years ago
|
||
Closing the Downloads manager window with Ctrl-E, Esc and the Downloads toolbar
button fails our basic functionality tests:
http://testrunner.mozilla.org/test.cgi?run_id=243#892
Comment 36•21 years ago
|
||
l10n changes please land soon.
Flags: blocking-aviary1.0RC1?
Flags: blocking-aviary1.0RC1+
Flags: blocking-aviary1.0-
Flags: blocking-aviary1.0+
Assignee | ||
Updated•21 years ago
|
Attachment #153186 -
Flags: approval-aviary?
Comment 37•21 years ago
|
||
Comment on attachment 153186 [details] [diff] [review]
toggle v1.0.4 (toolkit locale move)
Please don't request approval until you've received necessary reviews. Thanks.
Attachment #153186 -
Flags: approval-aviary?
Assignee | ||
Updated•20 years ago
|
Whiteboard: http://testrunner.mozilla.org/test.cgi?run_id=244#892 → [have patch]
Comment 38•20 years ago
|
||
Sorry to throw a spanner in the works but Ctrl+Y is the traditional windows
shortcut for Redo Change/Editing (Edit->Redo). Is assigning Ctrl+Y to the
download manager an oversight or has the functionality been overloaded
intentionally.
Assignee | ||
Comment 39•20 years ago
|
||
Manoj, bug 233440 changed the key from Ctrl+E to Ctrl+Y. Redo still works in
textareas and in the bookmarks manager. It doesn't work in the bookmarks
sidebar, but it never worked there.
Assignee | ||
Updated•20 years ago
|
Priority: P4 → P3
Comment 40•20 years ago
|
||
Oops. Steffen, can you remove the section that closes the dlmgr from the browser
window when the dlmgr isn't focused? The correct behavior when the window is
open in the background when Ctrl+Y is pressed from a browser window is to bring
it forward. We weren't talking about changing that - only allowing it to be
closed with Ctrl+Y from the dlmgr window.
As for the key collision.. oops. How about we change it back to E... but add a
Shift modifier. Ctrl+Shift+E.
Assignee | ||
Comment 41•20 years ago
|
||
Comment on attachment 153186 [details] [diff] [review]
toggle v1.0.4 (toolkit locale move)
no toggle then.
Attachment #153186 -
Attachment is obsolete: true
Attachment #153186 -
Flags: review?(mconnor)
Assignee | ||
Comment 42•20 years ago
|
||
Ctrl+Shift+E is dangerous because it's next to Ctrl+Shift+W, which is "close the
current window without prompting".
Ctrl+Shift+Y is a bit difficult to press with one hand on non-German keyboards.
Ctrl+Shift+D would be nice, but doesn't seem to work!?
Summary: Download Manager should close on CTRL+Y / ESC / toolbar button → Download Manager should close on CTRL+Y / ESC
Assignee | ||
Comment 43•20 years ago
|
||
This is mostly the first patch, plus ESC.
Assignee | ||
Comment 44•20 years ago
|
||
Comment on attachment 155752 [details] [diff] [review]
just make make Ctrl-y and ESC close it
Explanation is in comment 11 and comment 42.
Attachment #155752 -
Flags: review?(bugs)
Assignee | ||
Comment 45•20 years ago
|
||
"Note that you cannot use Shift-Ctrl-A-thru-F or Shift-Ctrl-0-thru-9 for your
own purposes, as these combinations are used to enter unicode characters in text
fields."
http://developer.gnome.org/projects/gup/hig/2.0/input-keyboard.html#shortcuts
Comment 46•20 years ago
|
||
Comment on attachment 155752 [details] [diff] [review]
just make make Ctrl-y and ESC close it
r+a=ben@mozilla.org
Attachment #155752 -
Flags: review?(bugs)
Attachment #155752 -
Flags: review+
Attachment #155752 -
Flags: approval-aviary+
Comment 47•20 years ago
|
||
(Ctrl+Shift+D is "add bookmark silently")
Assignee | ||
Comment 48•20 years ago
|
||
Checked in br & trunk 08/15/2004 05:59.
(In reply to comment #47)
> (Ctrl+Shift+D is "add bookmark silently")
Huh? Not in my tree.
Updated•16 years ago
|
Product: Firefox → Toolkit
You need to log in
before you can comment on or make changes to this bug.
Description
•