110517 - ctrl-k doesn't work in mail/news composer on unix

Status: VERIFIED FIXED

(MailNews Core :: Composition, defect)

Description

[Christopher Blizzard (:blizzard)]

This is present in both the 0.9.6 branch builds and on the tip as near as I can
tell.

In composer on unix if you hit ctrl-k in the middle of a line it should delete
all of the text to the end of the line.  Right now in composer, this isn't
working.  It does work in text areas.

If I change the binding in platformHTMLBindings.xml to '8' instead of 'k' it
does work.  I've tried to find conflicts with other keys and I've removed just
about every other 'k' instance that I have been unable to find it.

Comment 1

[R.K.Aa.]

i don't think there's any intention of fixing this - see bug 54399,
comment #13 

Comment 2

[Sheela Ravindran]

Sairuh,
Is this a bug a dup of 54399?

Comment 3

[Christopher Blizzard (:blizzard)]

No, this should work.

Comment 4

[Christopher Blizzard (:blizzard)]

That's about ^k on windows.  This is about ^k on linux which should work in
composer ( it works in all the other text widgets! ) There is a definite bug
here that needs to be addressed.

Comment 5

[Akkana Peck]

accel-K (XUL binding, I think) brings up the spellchecker in commercial builds.  
This conflicts with ctrl-K (XBL binding) for delete-to-end.  XBL bindings are
supposed to take precedence over XUL bindings, but they don't always (see bug
77976).  

Of course, redefining the accel key to be alt should cure that problem.

In moz builds, there is no spellchecker, so I would hope that we leave that
binding undefined, in which case ctrl-K should delete to end regardless of the
accel key setting.  If accel-K is doing a nonexistant function yet still
overriding another, useful, binding, that should be fixed.

Comment 6

[Akkana Peck]

Incidentally, the editor is not a text widget (in response to the use of the
word "other" in blizzard's last comment), even the plaintext editor.

Comment 7

[Christopher Blizzard (:blizzard)]

I tried changing the keybinding for the spellchecker to something else but it
didn't fix the problem.

Comment 8

[Christopher Blizzard (:blizzard)]

So, is there an easy way to track the key events as they are sent?  Maybe a way
to tell what is bound to what?

I know that if you bind '8' to the command handler that the code in
nsXBLPrototypeHandler::ExecuteHandler that sends the command to the handler is
executed.  However, if the 'k' key is bound to it it never takes that code path.

What are the other code paths are there for bound handlers other than that?

Comment 9

[Christopher Blizzard (:blizzard)]

OK, it is the spell checking binding.  I'm lame.

Index: editor/ui/composer/locale/en-US/editorOverlay.dtd
===================================================================
RCS file: /cvsroot/mozilla/editor/ui/composer/locale/en-US/editorOverlay.dtd,v
retrieving revision 1.94
diff -u -r1.94 editorOverlay.dtd
--- editorOverlay.dtd   2001/11/15 15:28:10     1.94
+++ editorOverlay.dtd   2001/11/17 02:48:46
@@ -118,7 +118,7 @@
 <!ENTITY validateCmd.label "Validate HTML">
 <!ENTITY checkLinksCmd.label "Check Links">
 <!ENTITY editcheckspelling.accesskey "s">
-<!ENTITY editcheckspelling.keybinding "k">
+<!ENTITY editcheckspelling.keybinding "8">
 
 <!-- View menu items -->
 <!ENTITY viewToolbarsMenu.label "Show/Hide"> 
Index: mailnews/compose/resources/locale/en-US/messengercompose.dtd
===================================================================
RCS file:
/cvsroot/mozilla/mailnews/compose/resources/locale/en-US/messengercompose.dtd,v
retrieving revision 1.59
diff -u -r1.59 messengercompose.dtd
--- messengercompose.dtd        2001/10/01 05:00:33     1.59
+++ messengercompose.dtd        2001/11/17 02:48:47
@@ -108,7 +108,7 @@
        <!ENTITY selectAddressCmd.accesskey "c">
 
 <!ENTITY checkSpellingCmd.label "Check Spelling...">
-       <!ENTITY checkSpellingCmd.key "K">
+       <!ENTITY checkSpellingCmd.key "8">
        <!ENTITY checkSpellingCmd.accesskey "S">
 
 <!ENTITY priorityMenu.label "Priority">

Comment 10

[Christopher Blizzard (:blizzard)]

ctrl-k is a completely legitimate text editor binding for unix so the spell
checking binding has to change, at least for unix.

How do we go about doing that?

Comment 11

[was]

This *does* work if you reply to a message, at least on 0.9.6 on Solaris. It
doesn't work with a new message.

Comment 12

[Jean-Francois Ducarroz]

-->varada

Comment 13

[David Juran]

Then it's broken again in 0.9.7 on Linux )-: Doesn't work no matter if you reply
to a mail or type a new one.

Comment 14

[(not reading, please use [e-mail] instead)]

taking all of varada's bugs.

Comment 15

[Boris Zbarsky [:bzbarsky]]

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

Comment 16

[Matthias Versen [:Matti]]

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

Comment 17

[Stefan Borggraefe]

Attachment: Changes the spellchecker keybinding to Ctrl-Shift-K

Ok, XUL bindings still overwrite XBL bindings (bug 71779). But even when this
was fixed, the key for invoking the spellchecker would still conflict with the
kill-line command. So after bug 71779 is fixed Check Spelling will have no
keybinding. 

Furthermore since Mozilla is now shipped with a spellchecker by default (see
recent dupes of this bug) and there is no fix for bug 71779 in sight, I think
it is better to change the keybinding for Check Spelling now, so Ctrl+K invokes
kill-line again.

Comment 18

[(not reading, please use [e-mail] instead)]

your patch changes the binding to ctrl + shift + k for all platforms.

I think it should remain ctrl + k (on win32, like it has been since Netscape
4.x, and probably earlier)

Let me find out what it was for Netscape 7.x and Netscape 4.x on linux, as it
would make sense to keep things as it was, for existing users, if we can and if
it still makes sense.

Comment 19

[beanladen]

ctrl-k on unix was always 'kill line after cursor' in many applications as it was
until spellchecker was shipped with mozilla. I think there's no question that it
must be changed back again. There's no reason to assume that users are already
"traditionally" expect ctrl-k to launch the spellchecker on unix. Don't look for
excuses not to fix this bug now, you may take your time to find out, but we 
already know that this is wrong now.

Comment 20

[(not reading, please use [e-mail] instead)]

it might be that it was ctrl + k on Netscape 7.x for Linux (so that had the same
bug as mozilla 1.x has now) and possibly no key combo on Netscape 4.x.

I'll double check.

note, the html composer (that comes with the app suite) has the same bug, right?

so we'd want to fix both html composer and mail compose to use ctrl+shift+k on linux

Comment 21

[(not reading, please use [e-mail] instead)]

> Don't look for excuses not to fix this bug now, 
> you may take your time to find out, but we 
> already know that this is wrong now.

You misread my comments.

I have no intention of changing what ctrl+k is on linux.

I want to find something other than that on linux (like ctrl+shift+k), to launch
the spell checker.

for win32, it will remain ctrl+l

Comment 22

[(not reading, please use [e-mail] instead)]

> ctrl+l

ctrl+k

Comment 23

[timeless]

Comment on attachment 136926 [details] [diff] [review]
Changes the spellchecker keybinding to Ctrl-Shift-K

as seth said this changes the windows binding which is unacceptable

Comment 24

[beanladen]

This bug changed the binding on unix for ctrl-k (kill line) beginning with 
Mozilla 1.5. THAT is unacceptable !

Comment 25

[Boris Zbarsky [:bzbarsky]]

I agree.  I've been forced to completely stop using mailnews on Linux because
any time I try to delete a line I get the spellchecker (which I don't use and
don't see any way to disable).  This is a pretty serious usability regression
that we should really try to fix for 1.6.

Comment 26

[Boris Zbarsky [:bzbarsky]]

Also, please note
http://www.mozilla.org/projects/ui/accessibility/mozkeyintro.html#modifiers
(under "Emacs editor bindings always go on Ctrl"):

  "On some letters, there is an accel binding, and a ctrl(unix) binding. When
   these two bindings conflict (as in ctrl-A or ctrl-H), the emacs binding wins."

There's a reason we have that rule...

Comment 27

[Brendan Eich [:brendan]]

Let's get a patch that fixes Unix only, please, for 1.6.

/be

Comment 28

[Boris Zbarsky [:bzbarsky]]

Attachment: timeless' proposed patch

Timeless proposed this... mostly his, with a bit of cleanup to make it work by
me, so neither of us can review.

Comment 29

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 136999 [details] [diff] [review]
timeless' proposed patch

Neil, Seth, what do you think?	I did test this on Linux and it works great.

Comment 30

[Kathleen :Brade]

Comment on attachment 136999 [details] [diff] [review]
timeless' proposed patch

r=brade but I think the message compose xul should have the command disabled by
default like it is in Composer.

Comment 31

[Scott MacGregor]

over to bz...

Comment 32

[Boris Zbarsky [:bzbarsky]]

Er... I just attached the patch on timeless's request since he had no real build
access and hence could not test.  I suppose I could check the patch in if/when
it gets reviews, but I can't guarantee I'll be able to do that after tomorrow.

Comment 33

[Scott MacGregor]

Comment on attachment 136999 [details] [diff] [review]
timeless' proposed patch

looks good, and works on win32.  sr/a=sspitzer, pending review from neil

Comment 34

[Scott MacGregor]

> sr/a=sspitzer

he gave the sr/a= over aim.

Comment 35

[[e-mail]]

Comment on attachment 136999 [details] [diff] [review]
timeless' proposed patch

fyi, message compose uses a different enabling style to html composer.

Comment 36

[timeless]

i checked this in yesterday. i'd like someone to verify (using a unix build from
the trunk that's from today or later...) that it's fixed.

Comment 37

[Stefan Borggraefe]

Verified with a new CVS build on Linux.

The shortcut for "Check Spelling" is now Ctrl+Shift+K. Ctrl+K invokes kill-line.
Tested in both Composer and the MailNews Compose window.

Comment 38

[Le,Sandy Sun]

It will conflicted with the hotkey of 'remove links'!

(In reply to comment #37)
> Verified with a new CVS build on Linux.
> 
> The shortcut for "Check Spelling" is now Ctrl+Shift+K. Ctrl+K invokes kill-line.
> Tested in both Composer and the MailNews Compose window.