Closed Bug 198976 Opened 22 years ago Closed 22 years ago

Ctrl+Enter (Ctrl-Enter) doesn't work in Compose window

Categories

(MailNews Core :: Composition, defect)

defect
Not set
major

Tracking

(Not tracked)

VERIFIED FIXED
mozilla1.4alpha

People

(Reporter: pratik.solanki, Assigned: bryner)

References

Details

(Keywords: access, regression, Whiteboard: [adt1])

Attachments

(2 files)

I looked and I looked but couldn't find a dupe. In today's build (build 2003032322), Ctrl+Enter in compose does not Send message like its supposed to.It just places a new line like regular enter.
Also affecting Windows XP in 2003032404. Some additional info: ctrl-enter still works in the addressing and subject controls. Only the message editing control is affected.
Marking All/All and raising severity to get it on people's radar. Also asking for blocking1.4a flag.
Severity: normal → major
Flags: blocking1.4a?
OS: Linux → All
Hardware: PC → All
It actually works in "To" field, does not work in the composing area.
*** Bug 199062 has been marked as a duplicate of this bug. ***
*** Bug 199103 has been marked as a duplicate of this bug. ***
I failed to find a dupe even though I thought I searched for ctrl and enter.. Oh well. Anyway, I suspect this is related to bug 54036.
*** Bug 199147 has been marked as a duplicate of this bug. ***
I see this too. any ideas when this broke?
Assignee: ducarroz → sspitzer
Target Milestone: --- → mozilla1.4alpha
*** Bug 199211 has been marked as a duplicate of this bug. ***
See also, bug 199068
Flags: blocking1.4a? → blocking1.4a-
In response to comment 8, it happened at some point between the 21MAR03 and 24MAR03 builds, because I installed the former last Friday and it didn't have this issue. But since that build had the really bad no-highlighting regression, I reinstalled on Monday to get the fix and this bug had shown up.
bryner says this was caused by his fix for bug #54035
Assignee: sspitzer → bryner
Keywords: nsbeta1
I know what's going wrong, but I don't know the HTML editor code well enough to say what the right fix is. We're hitting the case in nsHTMLEditor::HandleKeyPress where it calls PreventDefault() on the event if it's the return or enter key. That's happening prior to the <key> handler running, so XBL sees that preventDefault has been called and doesn't run the key handler. Should this code be checking if ctrl is pressed and not call preventDefault if that's the case? Simon? Kathy?
Shouldn't it be in release notes since it not gonna make 1.4a?
Keywords: relnote
for people who like to do things with keyboard shortcuts/accelerators, the workaround is shift+tab ctrl+enter the shift+tab moves focus to the subject line, and cltr+enter works from there. this might be worth adding to the release notes?
Or alternatively: ctrl+tab ctrl+enter since this involves less finger movement.
Attached patch Proposed patch (deleted) — Splinter Review
Attachment #118664 - Flags: superreview?(sfraser)
Attachment #118664 - Flags: review?(brade)
Just my opinion, but attachement 118664 seems like a hack to me. Also, it doesn't look to me like it will fix bug 199068, which is a related regression caused by the fix for bug 54035.
Comment on attachment 118664 [details] [diff] [review] Proposed patch >+ && !ctrlKey && !altKey && !metaKey) Why the altKey and metaKey checks as well? We don't have any Alt+Enter shortcuts do we?
> We don't have any Alt+Enter shortcuts do we? I don't want to break bug 76925
Comment on attachment 118664 [details] [diff] [review] Proposed patch I really don't think this is the right fix but I'm ok with it. It should have a comment explaining why we have to have it (with this bug #).
*** Bug 199602 has been marked as a duplicate of this bug. ***
I don't know all that much about the Mozilla codebase, and am just writing this comment as an experienced architect, so please take this suggestion with a small grain of salt... That said, my reading of bug #54035 makes me think that the correct fix for this bug might be to make sure the listener that's supposed to handle Ctrl-Enter in the editor gets put into the right "group", or perhaps is inserted in the right order. I'm also suspicious of the line in nsEditorEventListeners.cpp in attachment 113863 [details] [diff] [review] that checks "if (isSystemPass)" before calling "textEditor->HandleKeyPress(keyEvent);".
mail triage: nsbeta1+,adt1
Keywords: nsbeta1nsbeta1+
Whiteboard: [adt1]
Well, the reason this bug happens is that previously, editor was the very last listener to see keypress events, because it was all by itself in the system event group. Now, XUL <key> listeners also get put in the system event group, and they'll fire after editor's listener. So the question is, in the absence of a window-specific ctrl+enter key binding, should ctrl+enter insert an empty line into the editor? If so, then you're right, this isn't the best fix. In that case I think we need to do something else to make sure that the compose window keybinding is able to get the event before the editor listener. If ctrl+enter should _never_ type an empty line, then I think this fix is great. I just tested on Windows and Linux and found that ctrl+enter usually acts just like Enter, so I suspect the first option is the best fix.
I'm also concerned about what other regressions there may be because we rely on the editor getting the events after the XUL keybindings. Can we change that order easily? In some applications meta-enter inserts a page break or similar behavior (<p> vs <br>). I do think Composer should be able to handle that keybinding if the app doesn't grab it first.
Just to muddy the waters here, do the XUL keybindings fire after textboxes? MailNews has this annoying problem where they have to decide whether a 'N' key should type into a textbox or advance to the next unread message.
Attached patch alternative fix (deleted) — Splinter Review
This fix moves handling of all editor keypress events to the system event group, including modifier key combinations. I believe this is safe now since XUL key handlers have been moved. I've done some basic testing with this patch (ctrl+enter in mail compose, various menu shortcuts with focus in the URL bar in navigator, and menu shortcuts in Composer). There could be other cases out there though that this breaks. Anyone know of any?
Attachment #118844 - Flags: superreview?(sfraser)
Attachment #118844 - Flags: review?(brade)
You should probably check the following key-commands in ChatZilla (from bug 199068): Tab (completes channels and nicknames) Ctrl+PageDn/Ctrl+Tab (move to next tab) Ctrl+PageUp/Ctrl+Shift+Tab (move to previous tab) All these work from the text input at the bottom, and all got broken by bug 54036.
This fix actually relates only to modifier keys. Chatzilla's tab completion was (I think incorrectly) relying on the undocumented order where a window.onKeyPress handler would fire _after_ a <key> handler.
Well, the other two sets of key-commands stand as things that need testing. And what order are the events *meant* to fire? I was always of the understanding that events 'bubbled' up the DOM tree... where the <window> handlers *would* be reached _after_ the <key>s.
Oops. Meant 'broken by bug 54035' in comment 29. And I don't know how we're going to make CZ work now. :)
Related: bug 106327 - Ctrl+Return should be Ctrl+Enter bug 50255 - some control key sequences don't generate the correct event (ctrl-enter ...) This bug is possibly a duplicate of bug 50255. In any case, there has recently been work on these bugs that may have broken the behavior.
<key>s are sort of special; they aren't direct ancestors of the event target and yet they receive the event. They're actually implemented as event listeners on the window, so depending on the firing order of <key> and window.onKeyPress seems risky.
*** Bug 199934 has been marked as a duplicate of this bug. ***
What implications does the second patch have for midas and type-ahead find etc in browser windows?
Shouldn't have any impact on anything except modifier keys, and I tested ctrl+key combinations in the browser with focus in <input>s and found no problems.
What is the impact in mail (if any) when the users do commands like "m" for "Mark as read" or pressing space in input fields in browser (which in the past sometimes causes the space to be inserted as well as scrolling down a page)?
Why would there be any impact on mark as read? An editor isn't even focused in that case. It also only has the potential to affect keypresses that don't have a charcode. Space would have a charcode, so it's completely unaffected (I also tested this case to make sure it works).
I've seen problems (in N7?) where when I typed in the search box in the mail window, I couldn't type 'm' or 'r' because the keybinding was grabbing the key (similar to the problems seen with typeaheadfind). Thanks for checking the spacebar typing vs scrolling stuff. I'm very paranoid about that because it was SOooo annoying when that was broken.
Attachment #118664 - Flags: superreview?(sfraser)
Attachment #118664 - Flags: review?(brade)
Attachment #118664 - Flags: review-
Kathy -- I think you may have been seeing issues related to bug 195979, where the native menus grab unmodified 'm' and 'r' shortcuts before anything else gets them. That's atthe Carbon menu level, before it gets into the event system at all.
Comment on attachment 118844 [details] [diff] [review] alternative fix r=brade
Attachment #118844 - Flags: review?(brade) → review+
Attachment #118844 - Flags: superreview?(sfraser) → superreview+
*** Bug 200182 has been marked as a duplicate of this bug. ***
checked in.
Status: NEW → RESOLVED
Closed: 22 years ago
Resolution: --- → FIXED
*** Bug 200244 has been marked as a duplicate of this bug. ***
*** Bug 200251 has been marked as a duplicate of this bug. ***
*** Bug 200259 has been marked as a duplicate of this bug. ***
It's broken in 1.4a (3003040105) on Win2k. Please reopen this bug.
It's fixed in the main trunk, not in the 1.4a branch (yet?). Try 1.4b or today's nightly if they are out. 1.4a release was just one day before checkin of this fix.
*** Bug 200406 has been marked as a duplicate of this bug. ***
*** Bug 200513 has been marked as a duplicate of this bug. ***
*** Bug 200522 has been marked as a duplicate of this bug. ***
*** Bug 200562 has been marked as a duplicate of this bug. ***
*** Bug 200628 has been marked as a duplicate of this bug. ***
*** Bug 200670 has been marked as a duplicate of this bug. ***
*** Bug 200987 has been marked as a duplicate of this bug. ***
one note on the ctrl-tab or shift-tab trick suggested here: it doesn't work if you happen to be on a line with a bullet.
*** Bug 201144 has been marked as a duplicate of this bug. ***
*** Bug 201208 has been marked as a duplicate of this bug. ***
*** Bug 201516 has been marked as a duplicate of this bug. ***
still exists in the latest nightly for 1.4a: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.4a) Gecko/20030401
Trunk build 2003-04-10: WinXP, Mac 10.1.5 Trunk build 2003-04-08: Linux RH 7 Mozilla build 2003-04-10 nightly build: WinXP Verified Fixed.
Status: RESOLVED → VERIFIED
QA Contact: esther → nbaca
*** Bug 201688 has been marked as a duplicate of this bug. ***
*** Bug 202671 has been marked as a duplicate of this bug. ***
*** Bug 202962 has been marked as a duplicate of this bug. ***
*** Bug 203082 has been marked as a duplicate of this bug. ***
*** Bug 203112 has been marked as a duplicate of this bug. ***
*** Bug 203407 has been marked as a duplicate of this bug. ***
*** Bug 203494 has been marked as a duplicate of this bug. ***
*** Bug 203647 has been marked as a duplicate of this bug. ***
*** Bug 203647 has been marked as a duplicate of this bug. ***
*** Bug 203708 has been marked as a duplicate of this bug. ***
Maybe adding this will make it easier for people to find this bug before submitting a new one.
Summary: Ctrl+Enter doesn't work in Compose window → Ctrl+Enter (Ctrl-Enter) doesn't work in Compose window
*** Bug 203849 has been marked as a duplicate of this bug. ***
*** Bug 203965 has been marked as a duplicate of this bug. ***
*** Bug 204079 has been marked as a duplicate of this bug. ***
Product: MailNews → Core
Product: Core → MailNews Core
Keywords: relnote
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: