Closed
Bug 452634
Opened 16 years ago
Closed 13 years ago
Add keyboard shortcut to add attachment [Ctrl+Shift+A], change keyboard shortcut for "Remove Named Anchors" to [Ctrl+Shift+R]
Categories
(Thunderbird :: Message Compose Window, enhancement)
Thunderbird
Message Compose Window
Tracking
(thunderbird3.0 wontfix)
RESOLVED
FIXED
Thunderbird 10.0
Tracking | Status | |
---|---|---|
thunderbird3.0 | --- | wontfix |
People
(Reporter: hendrik, Assigned: squib)
References
(Blocks 2 open bugs, )
Details
Attachments
(1 file, 3 obsolete files)
(deleted),
patch
|
standard8
:
review+
neil
:
superreview+
squib
:
ui-review+
|
Details | Diff | Splinter Review |
There is keyboard shortcut to add an attachment when creating a new message. This forces me to use the mouse, which is annoying. It would be great if there were a keyboard shortcut.
Workaround: Use the accel keys for File → Attachment → File
Comment 1•16 years ago
|
||
Seconded by me.
Comment 2•16 years ago
|
||
Ping.
Updated•15 years ago
|
Version: 2.0 → Trunk
Comment 5•15 years ago
|
||
any shortcut proposals?
Reporter | ||
Comment 6•15 years ago
|
||
How about Ctrl+Shift+A? (Seems to do nothing now.)
Comment 7•15 years ago
|
||
That's taken. (Remove named anchors.)
Comment 8•15 years ago
|
||
I'm actually working on this bug. How about Cltrl_Shift+F, it's not taken elsewhere
Comment 9•15 years ago
|
||
(In reply to comment #8)
> I'm actually working on this bug. How about Cltrl_Shift+F, it's not taken
> elsewhere
Bryan ?
Assignee: nobody → mirna.bouchra
Comment 10•15 years ago
|
||
hmm, that looks like it's free to me though it doesn't have as much meaning as something like ctrl+shift+a would.
Comment 11•15 years ago
|
||
ctrl+shift+a is taken though
Comment 12•15 years ago
|
||
Attachment #418363 -
Flags: ui-review?(clarkbw)
Attachment #418363 -
Flags: superreview?(clarkbw)
Attachment #418363 -
Flags: review?(clarkbw)
Comment 13•15 years ago
|
||
Comment on attachment 418363 [details] [diff] [review]
The attached patch is to create a keyboard shortcut for attach file in Message Compose Window. the shortcut is ctrl+shift+F since ctrl+shift+a is taken. the shortcut can be changed if there is a more
Thanks so much for the patch! Sorry I fell behind on this during the holidays but I'm back now.
Everything looks good here other than some indenting on the messengercompose.xul file which should be easy to fix.
I'm minusing because I think should be using ctrl+shift+a as our key combination. After looking into the function that is already using it I believe we should drop that key code in favor of this one. Remove named anchors is a by far less used feature than attaching files to an email so I can't see why it would get the better key combination.
From here all that is required is to remove the key code from the other action such that we can use it for attachments. Then if you add me for the ui-review of that patch I'll make sure we can get a proper code review of it as well.
With any luck we might be able to land this in the 3.0.1 release to get it out quickly.
Attachment #418363 -
Flags: ui-review?(clarkbw)
Attachment #418363 -
Flags: ui-review-
Attachment #418363 -
Flags: superreview?(clarkbw)
Attachment #418363 -
Flags: review?(clarkbw)
Comment 14•15 years ago
|
||
Setting this as assigned to Mirna and marking for wanted in the 3.0 point releases however since shortcuts are localized our odds are low that we'll be able to sneak this in.
Status: NEW → ASSIGNED
status-thunderbird3.0:
--- → wanted
Comment 15•15 years ago
|
||
s/low/absolutely zero/. You're lucky that formatRemoveNamedAnchors.key is only in editor/ui/, not in mozilla/ somewhere, so you only have to coordinate stealing ctrl+shift+A with SeaMonkey mail and SeaMonkey composer, not with Firefox and its schedule too, but still: things like this which absolutely positively should not be done or even thought about for a stable branch are exactly why we want 3.1 and later to be quick releases. The first thing that should come into your head is "we'll be able to land this for the 3.1 release to get it out quickly" not "how can we subvert l10n to get this into some people's hands in less than three years?"
Comment 16•15 years ago
|
||
I wouldn't say 'subvert' as much as "employ judicious use of smoke and mirrors" but you're completely correct that my world has changed to a long release view and should change back to our new world order.
Comment 17•15 years ago
|
||
Even in a long release view world, doing something like this which is just a little extra convenience isn't right for l10n hence wontfix for the 3.0 branch.
Comment 18•15 years ago
|
||
Just to note that Cmd-Shift-A is the shortcut used by Mail.App, Outlook doesn't appear to have such a shortcut.
Comment 19•15 years ago
|
||
I'm really sorry I fell behind this for a while. I was having my final exams. Anyways, I'll ge this fixed really soon and update you. Thanks a lot for your help :)
Comment 20•15 years ago
|
||
Attachment #422017 -
Flags: ui-review?(clarkbw)
Attachment #422017 -
Flags: superreview?
Attachment #422017 -
Flags: review?
Comment 21•15 years ago
|
||
(In reply to comment #20)
> Created an attachment (id=422017) [details]
> Patch for removeNamedAnchors shortcut removed and Alt+shift+A shortcut for add
> attachment
Don't forget to target reviewers too :-)
Updated•15 years ago
|
Attachment #422017 -
Attachment description: Patch for removeNamedAnchors shortcut removed and Alt+shift+A shortcut for add attachment → Patch for removeNamedAnchors shortcut removed and ctrl+shift+A shortcut for add attachment
Comment 22•15 years ago
|
||
(In reply to comment #21)
> (In reply to comment #20)
> > Created an attachment (id=422017) [details] [details]
> > Patch for removeNamedAnchors shortcut removed and Alt+shift+A shortcut for add
> > attachment
>
> Don't forget to target reviewers too :-)
Mr Ludovic, you turned my attention to some mistake=) the shortcut is ctrl+shift+A not alt+shift+A. I do really apologize for this.
I requested a ui review from Mr Bryan W. Clark. could you please tell me who else should I ask their review?
Thanks a lot and sorry again for the typo
Comment 23•15 years ago
|
||
Comment on attachment 422017 [details] [diff] [review]
Patch for removeNamedAnchors shortcut removed and ctrl+shift+A shortcut for add attachment
Magnus, can you take the review on this? I'm not sure about the superreview requirements here but I'd like to get Mnyromyr to take a look.
Attachment #422017 -
Flags: ui-review?(clarkbw)
Attachment #422017 -
Flags: ui-review+
Attachment #422017 -
Flags: superreview?
Attachment #422017 -
Flags: review?(mkmelin+mozilla)
Attachment #422017 -
Flags: review?
Comment 24•15 years ago
|
||
Karsten, is this something you'd want for suite as well?
Comment 25•15 years ago
|
||
Comment on attachment 422017 [details] [diff] [review]
Patch for removeNamedAnchors shortcut removed and ctrl+shift+A shortcut for add attachment
This patch does not apply:
patching file editor/ui/composer/content/editorOverlay.xul
This patch doesn't apply
patch: **** malformed patch at line 40: diff -r 4b63c4badb42 editor/ui/locales/en-US/chrome/composer/editorOverlay.dtd
Also a few notes:
- align the attributes properly with those on the line above
- use spaces only (mozilla style is no tabs)
For the next patch, ask sr from neil as iirc he owns /editor. If this isn't wanted there this change might be significantly harder to code.
Attachment #422017 -
Flags: review?(mkmelin+mozilla) → review-
Updated•14 years ago
|
Blocks: attachUXtracker
Comment 26•14 years ago
|
||
Mirna, thanks a lot for working on this patch; I'm looking forward to this feature. Do you think you can fix the technical problems of your patch that are mentioned in comment 11?
btw, in bugzilla.mozilla.org, we tend to use first names instead of Mr. ... to make everyone's life easier (or full names where needed for clarification, but still without "Mr.").
Comment 27•14 years ago
|
||
Mirna, are you still working on this patch? If yes, could you fix the technical problems of your patch and request superreview as per comment 25?
If not, please unassign yourself from this bug (assign to nobody) so that somebody else can pick it up.
Updated•14 years ago
|
Summary: Add shortcut to add attachment → Add keyboard shortcut to add attachment [Ctrl+Shift+A]
Comment 28•14 years ago
|
||
(In reply to comment #23)
> Comment on attachment 422017 [details] [diff] [review]
> Patch for removeNamedAnchors shortcut removed and ctrl+shift+A shortcut for add
> attachment
>
> Magnus, can you take the review on this? I'm not sure about the superreview
> requirements here but I'd like to get Mnyromyr to take a look.
Isn't Ctrl-Shift-A already taken by Edit -> Select -> Thread?
(http://support.mozillamessaging.com/en-US/kb/Keyboard+shortcuts#Message_functions)
(Or is it okay that a shortcut has one behavior when a compose window is open and another the rest of the time?)
Updated•14 years ago
|
Blocks: tbattacha11y
Comment 29•13 years ago
|
||
Jim, this little bug already has ui-review+ and a draft patch (with some rough edged explained in comment 25). Would be a great complement to round off your keyboard accessibility work in bug 630759, only that this one is limited to composer. Perhaps you could have mercy...
(In reply to comment #28)
> Isn't Ctrl-Shift-A already taken by Edit -> Select -> Thread?
> (http://support.mozillamessaging.com/en-US/kb/
> Keyboard+shortcuts#Message_functions)
> (Or is it okay that a shortcut has one behavior when a compose window is
> open and another the rest of the time?)
Yes, it's very OK for a shortcut to have different behaviours in different contexts, especially different components (compose vs. mail reader).
No problem with Ctrl+Shift+A in the composer, that's why it has ui-review+.
Assignee: mirna.bouchra → nobody
Status: ASSIGNED → NEW
Whiteboard: [has ui-review+ and draft patch]
Assignee | ||
Comment 30•13 years ago
|
||
Here's an updated patch that should apply. Note that this doesn't work quite right in Linux; that's bug 269228.
Assignee: nobody → squibblyflabbetydoo
Attachment #418363 -
Attachment is obsolete: true
Attachment #422017 -
Attachment is obsolete: true
Status: NEW → ASSIGNED
Attachment #546063 -
Flags: ui-review+
Attachment #546063 -
Flags: superreview?(neil)
Attachment #546063 -
Flags: review?(mkmelin+mozilla)
Comment 31•13 years ago
|
||
Comment on attachment 546063 [details] [diff] [review]
Make Ctrl+Shift+A add attachment instead of remove named anchors
>diff --git a/mail/components/compose/content/editorOverlay.xul b/mail/components/compose/content/editorOverlay.xul
Given that you have your own override of this file, you could in theory just drop the removenamedanchorskb key locally. Or you could reduce disruption by renaming instead of removing the named anchors key. We'd want you to use one that isn't used by either SeaMonkey's editor or message compose, of course.
Assignee | ||
Comment 32•13 years ago
|
||
(In reply to comment #31)
> Given that you have your own override of this file, you could in theory just
> drop the removenamedanchorskb key locally. Or you could reduce disruption by
> renaming instead of removing the named anchors key. We'd want you to use one
> that isn't used by either SeaMonkey's editor or message compose, of course.
Do you have a preference either way? I'm not picky.
Comment 33•13 years ago
|
||
(In reply to comment #32)
> Do you have a preference either way?
I guess my preference is for you to rename the key.
Comment 34•13 years ago
|
||
Neil, thank you for providing the information needed for Jim to finalize this useful patch! *omg, what a poorly disguised attempt to show my eagerness on seeing this in action* :)
Assignee | ||
Comment 37•13 years ago
|
||
Ok, it looks like Ctrl+Shift+R is free for "Remove named anchors". I couldn't find anything else that used it, at least.
Unfortunately, Ctrl+Shift+A doesn't work very well on Linux, since GTK eats that key combo in text fields (e.g. addressing and subject fields, but not the message body). See bug 269228 for that. This is sad news for Linux users, but it's not like things are getting any worse for them as a result of this patch, so maybe it's not a big deal. We could try to find an alternate keybinding (or fix bug 269228), though.
Attachment #546063 -
Attachment is obsolete: true
Attachment #560514 -
Flags: ui-review+
Attachment #560514 -
Flags: superreview?(neil)
Attachment #560514 -
Flags: review?(mbanner)
Attachment #546063 -
Flags: superreview?(neil)
Attachment #546063 -
Flags: review?(mkmelin+mozilla)
Updated•13 years ago
|
Attachment #560514 -
Flags: superreview?(neil) → superreview+
Comment 38•13 years ago
|
||
Comment on attachment 560514 [details] [diff] [review]
Give "Remove named anchors" a different key
>diff --git a/editor/ui/locales/en-US/chrome/composer/editorOverlay.dtd b/editor/ui/locales/en-US/chrome/composer/editorOverlay.dtd
> <!ENTITY formatRemoveNamedAnchors.label "Remove Named Anchors">
> <!ENTITY formatRemoveNamedAnchors.accesskey "R">
>-<!ENTITY formatRemoveNamedAnchors.key "A">
>+<!ENTITY formatRemoveNamedAnchors2.key "R">
Axel, do I remember something mentioned in the past about we should be keeping all these names in sync for the benefit of the l10n tools? (i.e. add '2' onto the accesskey and label as well)?
Comment 39•13 years ago
|
||
For accesskeys, it might. This is a control key, though, which aren't mapped in to the string by tools, so that's fine.
Comment 40•13 years ago
|
||
Mark, given comment 39 which addresses your comment 38, would that imply your review+ for this small and straightforward patch?
Updated•13 years ago
|
Attachment #560514 -
Flags: review?(mbanner) → review+
Updated•13 years ago
|
Keywords: checkin-needed
Whiteboard: [has ui-review+ and draft patch]
Updated•13 years ago
|
Summary: Add keyboard shortcut to add attachment [Ctrl+Shift+A] → Add keyboard shortcut to add attachment [Ctrl+Shift+A], change keyboard shortcut for "Remove Named Anchors" to [Ctrl+Shift+R]
Assignee | ||
Comment 41•13 years ago
|
||
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 10.0
Comment 42•13 years ago
|
||
Comment on attachment 560514 [details] [diff] [review]
Give "Remove named anchors" a different key
>diff --git a/editor/ui/locales/en-US/chrome/composer/editorOverlay.dtd b/editor/ui/locales/en-US/chrome/composer/editorOverlay.dtd
>--- a/editor/ui/locales/en-US/chrome/composer/editorOverlay.dtd
>+++ b/editor/ui/locales/en-US/chrome/composer/editorOverlay.dtd
>@@ -212,7 +212,7 @@
> <!ENTITY formatRemoveLinks.key "K">
> <!ENTITY formatRemoveNamedAnchors.label "Remove Named Anchors">
> <!ENTITY formatRemoveNamedAnchors.accesskey "R">
>-<!ENTITY formatRemoveNamedAnchors.key "A">
>+<!ENTITY formatRemoveNamedAnchors2.key "R">
Ouch. First, why change the string ID at all there? Second, now (L10n) tools cannot connect the key to the label and accesskey any more because the part before the . is different :(
Comment 43•13 years ago
|
||
Hmm, OK, Axel already weighed in. I guess for a .key it's tolerable, even if not ideal.
I hope that with L20n we'll get some tooling that will make stuff like that cleaner.
Updated•13 years ago
|
Keywords: checkin-needed
Updated•13 years ago
|
Blocks: tb-keyboard-tracker
Comment 44•13 years ago
|
||
This needs to be mentioned in relnotes (new in tb10), and updated in keyboard shortcuts article.
Blocks: tbkbd-doc-tracker
Keywords: relnote
Comment 45•13 years ago
|
||
(In reply to Thomas D. from comment #44)
> This needs to be mentioned in relnotes (new in tb10)
Jen has documented this in http://support.mozillamessaging.com/kb/new-thunderbird-10. Thank you!
Should this be mentioned in the actual relase notes, too? I think we should be more generous with mentioning new features, and adding attachments is quite a frequent action...
https://www.mozilla.org/en-US/thunderbird/10.0/releasenotes/ (not yet published, so final URL might change)
-> What's New in Thunderbird
Well, and there's still a third place:
Help > What's New
...with an TB-Internal documentation (opens in a TB-tab) of some highlights of the new version.
IMO, new keyboard shortcuts definitely belong there, and they should have their own section.
> and updated in keyboard shortcuts article (1)
I've updated the keyboard shortcuts article with this new shortcut already, in major revision 5658 (2), as documented in bug 721666.
|Remove Named Anchors||{for =tb31,=tb5,=tb6,=tb7,=tb8,=tb9}{for win,linux}{key Ctrl+Shift+A}{/for}{for mac}{key Command+Shift+A}{/for}{/for} {for tb10}{for win,linux}{key Ctrl+Shift+R}{/for}{for mac}{key Command+Shift+R}{/for}{/for}
|Attach File||{for tb10}{for win,linux}{key Ctrl+Shift+A}{/for}{for mac}{key Command+Shift+A}{/for}{/for}
(1) https://support.mozillamessaging.com/en-US/kb/keyboard-shortcuts/revision
(2) https://support.mozillamessaging.com/en-US/kb/keyboard-shortcuts/revision/5658
Blocks: 721666
You need to log in
before you can comment on or make changes to this bug.
Description
•