4.x Linux used Alt-Enter as shortcut for Send. I'd like to see this revived - I am used to it. Should now probably be Accel-Enter.

I think this might be mail window FE yes in nc4win used ctrl-enter, so it should be accel enter. This should be really easy to do. ducarroz i'm setting a milestone of m0.9 if you get to the point where you can't do this by then but that an outsider can, please reassign to me.

*** This bug has been marked as a duplicate of 20336 ***

Verified dupe.

Reopening. This bug will be focussed on the subject I set.

Stealing bug. This is now blocking the nonsense of bug 20336. Do not mark this bug as a duplicate. I am working on it.

I don't understand. This *is* a DUP. How do you intend to implement it?

Oh, and forget about getting this into N6.0. You'll have no chance to get approval.

That bug is focused on preventing this key binding from executing. messengercompose.xul [the actual key stuff does not work, i'm not sure why][no I would not leave the javascript inline, this is just code that would theoretically describe my intentions] <script id="drive_everyone_crazy"> if(getpref('peoplehate-4xp')){ unbindkey(document.getElementById('cmd_SendNow')); unbindkey(document.getElementById('cmd_SendLater')); } function unbindkey(menuitem){ menuitem.removeAttribute('key'); menuitem.removeAttribute('acceltext') } </script> <key id="key_sendNow" keycode="&sendNowCmd.keycode;" observes="cmd_sendNow" modifiers="accel"/> <key id="key_sendLater" keycode="&sendLaterCmd.keycode;" observes="cmd_sendLater" modifiers="accel,shift"/> <menuitem id="menu_SendNow" value="&sendNowCmd.label;" accesskey="&sendNowCmd.accesskey;" observes="key_sendNow"/> <menuitem id="menu_SendLater" value="&sendLaterCmd.label;" accesskey="&sendLaterCmd.accesskey;" observes="key_sendLater"/>

I still don't understand why 2 bugs. Why don't you take the other bug? *This* bug was about a "Keyboard shortcut for Send" (initial SUMMARY), not Ctrl-Enter specifically. And after seeing the discussion in the other bug, my suggestion is to use an F-key, *not* Ctrl-Enter.

not going to make it for NS 6.0 rtm.

Adding to "Upgrading to Netscape 6" guide

*** Bug 60983 has been marked as a duplicate of this bug. ***

Nominating for nsbeta1 since it is a frequent used feature and it is not here yet. :(

Changing kristif to robinf on cc: list as Robin Foster-Clark is now the Composer docs contact.

Couple of things (sorry this is a bit long): * We've been having keyboard UI meetings every other week to clear up confusion and set things straight. Email me if you'd like to get involved. * The results of our spec so far: www.mozilla.org/projects/ui/accessibility/mozkeyintro.html * We decided that accel combos should be used with letters and numbers, but everything else should be consistently ctrl, or alt. For this bug, that means we would use ctrl+enter. The reason for this are in the keyboard intro listed above, but briefly, alt+left and alt+right for prev/next page doesn't change when your accel key changes ... it's the modified letter and number commands that should change only. * Modified Enter and Space were listed in the FAQ/spec as keys that users frequently hit by mistake. Therefore they must not do anything destructive (such as send mail before asking). The result is this: we need a dialog box to pop up after Ctrl+Enter is pressed "Send Mail Now?" with a checkbox "Remember this decision" . People won't have to always see it if that's there preference. Please let me know if any of this doesn't jibe with you. - Aaron

*** Bug 73939 has been marked as a duplicate of this bug. ***

having a pref or dialogue is already being dealt with (extensively discussed) in bug 20336. This bug should finally implement the CTRL-ENTER feature :) It was in Netscape and is in M$ outlook/express - so potential "defectors" need to find their used-to environment.

I can't reasonably work on this until bug 76891 is fixed. Killing milestone. If people want to enable it on other platforms they can change the file to have the following lines: <key id="key_sendNow" keycode="&sendNowCmd.keycode;" observes="cmd_sendNow" modifiers="accel"/> <key id="key_sendLater" keycode="&sendLaterCmd.keycode;" observes="cmd_sendLater" modifiers="accel,shift"/> [lessimportant] <menuitem label="&sendNowCmd.label;" key="key_sendNow" accesskey="&sendNowCmd.accesskey;" observes="cmd_sendNow"/> <menuitem label="&sendLaterCmd.label;" key="key_sendLater" accesskey="&sendLaterCmd.accesskey;" observes="cmd_sendLater"/>

*** Bug 79325 has been marked as a duplicate of this bug. ***

bug 76891 was dup'd in favor of bug 50255, which is set for 0.9.3 :(

How does this differ from bug 70185 (which depends on this bug).

*** Bug 85428 has been marked as a duplicate of this bug. ***

I was playing with this but those <key> bindings didn't work. With 'keycode="VK_ENTER" modifiers="accel"', pressing Ctrl-Enter in the compose window just inserts a new line break into the editor content.

VK_RETURN works, VK_ENTER doesn't. I patched the editor to stop it from eating Ctrl-Enter etc. I set the second button to "Cancel". I think it's important to let the user know that that button is safe and nothing is going to happen. Given that, though, the Cancel button must ignore the "don't show again" check box, because Cancel always ignores the contents of the controls and never changes state. (This means you can't set "don't show again" with the action set to "don't send" so ctrl-enter is always silently ignored, but that doesn't seem particularly useful.)

I'm not familiar with this area of the system. Who should I ask for review/sr?

methinks seth, mscott or bienvienu would be able to sr this. cc'ing them.

*** Bug 89601 has been marked as a duplicate of this bug. ***

As a parity issue, other AOL products such as AIM use this feature (CTRL+Enter to send). Adding myself to CC list and putting in my vote.

I'm very much in favour of this, but if it's on a Mac then it *MUST* be command-enter not control-enter. Nothing uses the control key on the Mac. For those that don't know the command key is the one with the Apple or 'four-leaved-clover' symbol on it.

*** Bug 91151 has been marked as a duplicate of this bug. ***

*** Bug 91249 has been marked as a duplicate of this bug. ***

*** Bug 95820 has been marked as a duplicate of this bug. ***

I'm going to steal this bug from timeless since I have the fix in hand.

*** Bug 96547 has been marked as a duplicate of this bug. ***

*** Bug 97148 has been marked as a duplicate of this bug. ***

Robert, any progress?

*** Bug 98612 has been marked as a duplicate of this bug. ***

The patch is still good as far as I know. I will try again to get reviews.

I am concerned about the proposed changes to the editor files. Please do not check in any changes to editor files without first obtaining module owner permission/review from at least one editor person.

Should the mail.NoWarningBeforeSend pref have a default value in all.js? Also, it should most likely be named noWarningBeforeSend, to keep consistent capitalization.

Who would be the right editor person?

This brings the patch up to date and incorporates Dean's recommendations. Please review this patch.

Are both changes to "nsEditorEventListeners.cpp" required to fix this bug? What is the effect if those changes are not applied?

pref name style is actually supposed to be all lowercase with underbar separators, but apparently reviewers don't know that.

If the editor changes are not applied, then the composer edit control will treat "Ctrl-Enter" the same as "Enter" (i.e., insert a carriage return), totally breaking this feature.

PS, brade, from CVS blame it looks like YOU are the right editor person to review the editor changes :-). Let me clarify my earlier comment ... we don't need the fix for consuming TABs in order to fix this bug. But we might as well fix them both, right?

dveditz: I thought that was the case for naming prefs, yeah, but then I looked at the existing names in there and got confused. Thanks for the clarification.

I'd rather not see the tab key "fixed" with this bug. Please file a separate bug on that issue and assign it to me. I'm concerned about tab usage since I know that it will be changing in Composer (other bugs).

Will Crtl-Enter "send now" on all occurances or does it default to "send later" when in Offline mode? Or should that be filed as seperate RFE?

Whether or not it's a separate RFE, the key combo should behave differently in on vs off line mode because the "Send" toolbar button also changes behavior in the two modes. The key combo and the toolbar button should provide identical behavior, I say.

You're right. I'll update the patch soon. Good catch!

changes to editor/base/nsEditorEventListeners.cpp (patch v4) are r=brade

Can we avoid using double negatives? For example, change +pref("mail.no_warning_before_send", false); to: +pref("mail.warning_before_send", true); Makes a lot more sense here.

> +pref("mail.warning_before_send", true); "warn_before_send" would be better than "warning_..."

Now, if we're in offline mode, "ctrl-Enter" will do a 'send later'. The only problem is what to do with the shortcut in the menu since even in offline mode we still show 'ctrl-enter' next to 'Send now', even though it doesn't do that. It turns out to be impossible to change the shortcut text (without fixing some XUL bugs). I ameliorated the problem somewhat by fixing a bug so that in offline mode, "send now" is shown disabled. I also added "shift-ctrl-enter" which is always bound to "send later".

hrm, you could cheat and have 4 <menuitem/>s although no one would approve of that.

The last patch adds code that would fix the accelerator key text if not for a XUL bug, which has been filed as bug 99853 (for which I just rolled a different patch). We don't have to wait for 99853 to go in before we land this though, we'll just have slightly misleading accelerator text in the meantime. The last patch also fixes a bug in previous patches that wasn't setting the 'disabled' state of cmd_sendNow correctly.

Does everybody agree this should be nominated for the branch?

yup.

hey robert, thanks for taking on this beast. varada would be the right person to give you a review of the mailnews code (I'll help out with the sr= of the mailnews code) Taking a quick look at your patch, here are some comments: 1) +pref("mail.warn_on_send_accel_key", true); should be added to mailnews.js, not all.js 2) +var savedSendNowKey; convention has been to name globals gFoo. so this should be +var gSavedSendNowKey;

OK, so they say this doesn't work in Windows. Perhaps Windows sends VK_ENTER instead of VK_RETURN? varada, can you check that out?

Does patch #8 actually work on Windows? I have my doubts which is why I'd prefer to see patch #8 obsoleted and stick with patch #7. The problem with this not working on Windows for patch #7 is entirely bug #50255. If we need a keybinding to Send Message and we can't get bug #50255 fixed, then we should use a keybinding that will work... maybe Control-Shift-D?

... or how about CTRL+SHIFT+ALT+Space+DEL+S <-- NOT!!! The solution *must* be CTRL+ENTER! Anything else is just a cop-out.

It has to be Ctrl-Enter for consistency with lots of other applications. Anything else would be a total cop-out. I have not tried patch #7 or #8 on Windows. varada says that patch #7 doesn't work on Windows. No-one's tried #8 yet. I didn't know about bug 50255, but I do see that almost everywhere in the code VK_RETURN and VK_ENTER are mapped to do the same thing. That's why I suspect that some platforms generate VK_RETURN and others VK_ENTER. Or could someone explain what the difference between VK_ENTER and VK_RETURN is supposed to be?

I'm not into programming but the Enter key is the one on the numeric keyboard and the return is the one on the alphanumeric keyboard - Two different keys with usually the same function. I pretty sure that VK_ENTER and VK_RETURN deal with theese two keys.

See bug 50255 for information about windows and ctrl-enter. That should be fixed but I don't think it should block the checkin of the code from this bug.

I still wish to nominate patch #8. "Enter" and "Return" do the same thing in most other places in Mozilla and I'd like that to be the same here.

Besides, VK_ENTER is not even defined in winuser.h, e.g. So at least on Windows there are no two different things like that.

Joki and Saari are the folks to ask about the difference between Enter and Return in the DOM key event spec. Cc'ing for their words of wisdom.

> "Enter" and "Return" do the same thing The usually have the same effect but AFAIK Shift+Return == Enter (which i often use because it allows me to continue typing without moving my from the home keys to the numpad)

MScott - Is this for 0.9.4 branch?

This P3 enhancement is not a stop-ship, PDT-.

nsBranch-

Dissapointing... So many people would just love this feature.

I honestly don't care about the branch. I just want this checked into the trunk before it rots (AGAIN). Can I have review/sr for patch #8 please?

Yes, it could have been a model fix: the code's supplied, all the incessant bickering's finally over - how I do like Angry Mob Government - and we need but a primate to hit the big red button marked GO in order to implement it. Not this time, I guess - their gaze has been averted by something more shiny.

well maybe there's hope after all. I'm very confused. We have a dup of this bug which was the one I was paying attention to: 20336. That bug says that we needed a fix from Rod Spears in Bug #50255 in order to get this to work properly all the time. Rod wasn't going to get to 50255 before this current release. What's the difference between 20336 and 50255? Are we saying that Patch v8 in this bug solves everything? No non mailnews changes required? - a confused mscott

I state my continued objection to #8. It should only work for VK_ENTER. If we knew there was a problem and needed to add VK_RETURN, then we could do that but right now we don't and adding an extra keybinding for no reason just contributes to bloat.

bug #50255 is Windows-specific. The problem in #50255 is that this particular key combination (and a few others) are not generating events. If you want the specifics, read that bug or e-mail me. Bug #50255 is a general problem across the whole product. This bug and bug #20336 are specific to mail.

So according to the testcase here: http://bugzilla.mozilla.org/attachment.cgi?id=29692&action=view my laptop keyboard sends VK_RETURN but my home desktop machine keyboard sends VK_ENTER when I press the key labelled "Enter". (Both machines running Linux, but different distributions.) Just about everywhere else in Mozilla where we deal with "VK_ENTER" or "VK_RETURN", the code is written to accept both of them (and do the same thing with both). So I don't think Brade's objection is consistent with the rest of the codebase; everyone else has assumed that it is necessary to handle both. If you want to change all that, fine, but do we have to start here? I don't want to wait for bug 50255 to be fixed because the patch does work on Linux right now and that's what I'm using. Let's check this in and then someone who cares about Windows can fix 50255 for those users. (It's not going to hurt them, they'll just see a shortcut that they can't use.)

If the patch works, why not check it in? (Perhaps with duplicate lines for Enter and Return -- I wish we could get an answer for what the difference is in our event model.) Then as soon as the Windows folks fix the event handling, it will immediately work there too, instead of everyone having to wait for someone to resurrect this patch yet again?

Comment on attachment 50075 [details] [diff] [review] Patch (v8) --- added code to make both VK_ENTER and VK_RETURN work Get rid of the dump() in SendMessageWithCheck() and work out with brade the ENTER/RETURN stuff and you have an sr=blizzard

Well I take back one thing I said above ... on my home machine the Enter key sends RETURN as well. (The numeric keypad Enter sends RETURN too.) I still think we should follow the rest of the code and make VK_ENTER and VK_RETURN do the same thing. But since Brade isn't convinced of that then here's my suggestion: let's check in patch #7 (same as patch #8 but without VK_ENTER) (with the dump taken out). It won't work on Windows so file a bug about that which depends on bug 50255. Once 50255 is fixed either the new bug will also be fixed or we'll need to add the VK_ENTER lines ... either way it will be a small fix.

r=brade on the latest strategy (remove the dump, patch #7) Rather than filing a new bug, you might see if any of the dups on this bug are appropriate to use. Personally, I'd wait until #50255 is fixed before doing anything. That fix should work; if it doesn't, that bug should probably be reopened. The only problem with this keybinding that I would foresee would be with a non- U.S. keyboard (any OS) but the odds of that are fairly low as well. For the record, I don't think VK_ENTER is really used by anyone. On Mac, gtk, QT, BeOS, OS2, photon they are mapped the same (to VK_RETURN): http://lxr.mozilla.org/seamonkey/source/widget/src/mac/nsMacEventHandler.cpp#807 http://lxr.mozilla.org/seamonkey/source/widget/src/gtk/nsGtkEventHandler.cpp#178 and nsGtkEventHandler.cpp#224 http://lxr.mozilla.org/seamonkey/source/widget/src/qt/nsQEventHandler.cpp#51 http://lxr.mozilla.org/seamonkey/source/widget/src/beos/nsWindow.cpp#1789 / #1818 http://lxr.mozilla.org/seamonkey/source/widget/src/os2/nsWindow.cpp#3016 http://lxr.mozilla.org/seamonkey/source/widget/src/photon/nsWidget.cpp#1267 #1303 I don't see anything in the windows code since it is written completely differently but that code will be changing with bug #50255 anyways.

brade, you are an editor person, right? In which case I still need review from someone in mailnews... Thanks!

Comment on attachment 50029 [details] [diff] [review] Patch (v7) --- changed name of global, moved pref I have reviewed and tested it on Mac. Looks good. R=ducarroz BTW, as you are touching MsgCOmposeCommands.js, can you review the following comment which doesn't have sense anymore: // RICHIE: We should really have a way of using constants and not // hardcoded numbers for the first argument

OK, I believe that http://bugzilla.mozilla.org/attachment.cgi?id=51405 is ready to be checked in sr=blizzard r=ducarroz,brade. It removes the dump() from patch #7 and also removes the (other people's) comments mentioned by ducarroz. I will check it in ASAP.

Checked in. Anyone following this bug who uses Windows should move their attention to bug 50255.

Actually marking FIXED!

+ var warn; + try { + warn = prefs.GetBoolPref("mail.warn_on_send_accel_key"); + } catch (ex) { + warn = true; + } you don't need to worry about GetBoolPref() failing since you defined a default in mailnews.js you can turn that try / catch block into: var warn = prefs.GetBoolPref("mail.warn_on_send_accel_key");

thanks for fixing this bug, robert. (and thanks to ducarroz / blizzard for the reviews)

I haven't been able to test this out (due to #50255.) I'll get a screen shot (on linux) for jglick and robinf so that they can review the UI and the wording.

here's an ascii screen shot, for robinf / jglick (I'll get a better one after I install xv) (?) Are you sure you are ready to send this message? [ ] Do not show me this dialog box again. [Send] [Cancel]

Personally, I'm not sure i see the benefit of showing a confirmation dialog only when using an accelerator key, since the point of an accelerator is to make things faster, not add steps. A pref to turn off, or maybe change, the accelerator would take care of those worried about accidentially sending mail. But if folks want this, would recommend simplifying the text to: "Send the message now?" [Send] [Cancel]

I've fixed the .js to be: + var warn = prefs.GetBoolPref("mail.warn_on_send_accel_key");

jglick, the current dialog has a checkbox to allow the user to disable the confirmation dialog. [ ] Do not show me this dialog box again is currently hooked up to the "mail.warn_on_send_accel_key" pref. I think that showing the alert is a good idea to prevent the user from sending before they meant to. "Send the message now?", what about if we are offline? In that case, ctrl+enter is really going to be doing a "send later".

If the text is a problem, we can set things up so that we have one prompt text if we're online and different text if we're offline. If this is deemed necessary, please file a bug against me about it.

1. I agree with jglick - questioning the necessity for having confirmation dialog only in case of using shortcut. Yes, we can disable confirmation option by selecting/marking option: "Do not show me this dialog box again" on the "Send the message now?" dialog. Why not to have it as well when use 'File|Send Now'? I believe, implementation should be adequate when using menu item or shortcut. 2. I see on the recent trunk build: 'Ctrl+Return' instead of common 'Ctrl+Enter'. Is it the solution after long discussion? We have 'Enter' button on common keyboards, but we don't have 'Return'. Also Communicator 4.x has 'Ctrl+Enter'. (Macintosh keyboard has 'Return'). 3. Yes, it works on Linux. Not on Windows so far (50255).

> 1. I agree with jglick - questioning the necessity for having confirmation > dialog only in case of using shortcut. > Normally I agree but I think that the exception was made ( and rightfully so IMHO ) because it's so easy to trip over ctrl-enter as a hot key.

FWIW, I'm strongly against this dialog. Accelerator keys for the send operation should not be bringing up a confirmation dialog confirming the use of this accelerator key. That's just not right. 4.x used the same short cut and it never bubbled up as a top complaint (that would justify the dialog) amongst our feedback reports.

Well then, you've obviously never tripped over the ctrl-enter button (which is easy to do when you use ctrl and arrow keys to navigate around messages), sending an unfinished message to a prospective employer and making you seem like an all around idiot. Besides, you can A) turn off the dialog if you want, and B) hit spacebar (I assume) to confirm sending the message.

Before: accelerator didn't work at all. After: you press the accelerator, click on the check box, hit OK, and you never see the warning again ever. I call that progress. Please read all the comments in this bug and bug 20336 before you add any new comments, if you haven't already. We had all these arguments months ago. We settled on this solution and no-one objected in the last three months.

So, if I happen to use the shortcut key for close window, why do I get prompted then? I don't see how the Send Message shortcut is really different from the Close shortcut as far as prompting goes.

Three reasons: 1) We have anecdotal evidence that Ctrl-Enter is a particularly common combination to hit accidentally. 2) Accidentally closing a window is usually protected against dataloss (via history, or "Are you sure you want to discard these changes", etc)3) Accidentally sending a half-composed message is sometimes worse than dataloss.

There needs to be a way in prefs the user can toggle that pref back on, so that the dialog shows. For every pref like this, the use has to be able to get back to the original state.

People: This bug has been a LONG time coming. Most of us are *very* happy with the way it has been resolved. Let it go in as is and file a separate bug to address your nitpicks. The complaints are getting farther and farther off topic. Just let us have our shortcut!

I agree, the current state is an improvement. ctrl+enter works! (at least on linux). we can easily solve any UI problems. for example, if netscape decides to never show that prompt, we can override the pref in mailnews-ns.js. I agree with sfraser, it would be nice if we found a place in the prefs dialog to allow the user to turn it back on. I'll go spin off the existing UI issues to another bug.

UI spun to http://bugzilla.mozilla.org/show_bug.cgi?id=102643

> Well then, you've obviously never tripped over the ctrl-enter button (which is > easy to do when you use ctrl and arrow keys to navigate around messages), > sending an unfinished message ... Odd, ctrl + arrow keys doesn't seem to do anything. Is this a bug in Moz, a mismatch vs NS4, or a different mailer altogether+ FWIW, enter is far from the inverted T of my arrow keys :)

In bug 20336 I found: Comments From Jean-Francois Ducarroz 2000-10-15 11:26 ------- *** Bug 56696 has been marked as a duplicate of this bug. *** I verify duplicate of bug 20336 and fix on Linux with the trunk build 2001-10-01.

Okay everyone else, sorry for the spam. ctrl+left arrow or right arrow moves the cursor a complete word left or right (there is also a bug to get to get the cursor to stop at characters such as / and .) Ctrl+shift+arrows is a good way to select a bunch of text or a complete word. If you have selected text and you just start typing, it replaces the text you have selected with what you're currently typing. This is pretty handy when you're going over an important email and you make a lot of revisions :) Okay, from this point on I'm not trying to justify accidental ctrl-enter, this is just where one user is coming from. With Microsoft Word 97, if you select text and press enter, it just deletes the selection. For some stupid reason, I fancied this as the method of erasing stuff. Too lazy to hit backspace I guess. Write papers like this for several years and it becomes a habit. Then go into netscape, type up messages in a flurry, use ctrl-shift, then enter a lot, and you end up sending unfinished messages. Phew. So I dunno what platform you're on, but if you don't get the ctrl+arrows behavior, it's probably a bug.

It is misleading to see Verified Fixed - it is not fixed yet. According to comments #2, 3 the resolution to this bug could be Verified Duplicate, but we have such a history in here, so I'd rather re-open this. Other opinions? Related: bug 20336, bug 102643, bug 50255.

This bug is fixed. The original request was for the accel+Enter shortcut to send the message. It works on every platform, to my knowledge, except Windows (bug 50255).

Actually this is fixed. Control-Enter does work to send on Linux (and presumably also Macintosh). If this is not the case, then you can reopen the bug. If you are seeing a problem because you are on Windows, you should be commenting in bug #50255 (assuming you have anything relevant to add and no, "it still doesn't work" doesn't qualify as relevant). All Windows-OS users should be tracking bug #50255 if they are concerned about this keybinding not working. This bug should NOT be reopened for Windows-specific problem which is covered in bug #50255.

No problem, but: 1. This bug shows All OS (though short description mentions just Linux and dif shortcuts.) A lot of work, discussion, and changes have been done since then! 2. Linux works but shows Ctrl+Return, Ctrl+Shift+Return (Return instead of Enter), 3. Mac: shortcut works but is not shown as Ctrl+Return, Ctrl+Shift+Return (should be Return only on Mac), 4. Windows - not implemented. Bug 50255 has also history on this. No problem!