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

VERIFIED FIXED in mozilla1.6final

Status

VERIFIED FIXED
17 years ago
7 years ago

People

(Reporter: blizzard, Assigned: timeless)

Tracking

(Depends on: 1 bug)

Trunk
mozilla1.6final
x86
Linux
Dependency tree / graph
Bug Flags:
blocking1.6 +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Reporter)

Description

17 years ago
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

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

Comment 2

17 years ago
Sairuh,
Is this a bug a dup of 54399?
(Reporter)

Comment 3

17 years ago
No, this should work.
(Reporter)

Comment 4

17 years ago
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

17 years ago
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

17 years ago
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.
(Reporter)

Comment 7

17 years ago
I tried changing the keybinding for the spellchecker to something else but it
didn't fix the problem.
(Reporter)

Comment 8

17 years ago
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?
(Reporter)

Comment 9

17 years ago
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">
(Reporter)

Comment 10

17 years ago
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?

Updated

17 years ago
QA Contact: sheelar → esther

Comment 11

17 years ago
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.
-->varada
Assignee: ducarroz → varada

Updated

17 years ago
Status: NEW → ASSIGNED
Target Milestone: --- → Future

Comment 13

17 years ago
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.
taking all of varada's bugs.
Assignee: varada → sspitzer
Status: ASSIGNED → NEW
*** Bug 217877 has been marked as a duplicate of this bug. ***
Depends on: 77976
*** Bug 225712 has been marked as a duplicate of this bug. ***

Comment 17

15 years ago
Created attachment 136926 [details] [diff] [review]
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.

Updated

15 years ago
Attachment #136926 - Flags: superreview?
Attachment #136926 - Flags: review?

Updated

15 years ago
Attachment #136926 - Flags: superreview?(sspitzer)
Attachment #136926 - Flags: superreview?
Attachment #136926 - Flags: review?(sspitzer)
Attachment #136926 - Flags: review?
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.
Target Milestone: Future → mozilla1.6final

Comment 19

15 years ago
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.
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
> 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
(Assignee)

Comment 23

15 years ago
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
Attachment #136926 - Flags: superreview?(sspitzer)
Attachment #136926 - Flags: review?(sspitzer)
Attachment #136926 - Flags: review-

Comment 24

15 years ago
This bug changed the binding on unix for ctrl-k (kill line) beginning with 
Mozilla 1.5. THAT is unacceptable !
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.
Flags: blocking1.6?
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...
Let's get a patch that fixes Unix only, please, for 1.6.

/be
Flags: blocking1.6? → blocking1.6+
Created attachment 136999 [details] [diff] [review]
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.
Attachment #136926 - Attachment is obsolete: true
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.
Attachment #136999 - Flags: superreview?(sspitzer)
Attachment #136999 - Flags: review?(neil.parkwaycc.co.uk)

Comment 30

15 years ago
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

15 years ago
over to bz...
Assignee: sspitzer → bz-vacation
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

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

looks good, and works on win32.  sr/a=sspitzer, pending review from neil
Attachment #136999 - Flags: superreview?(sspitzer)
Attachment #136999 - Flags: superreview+
Attachment #136999 - Flags: approval1.6b+

Comment 34

15 years ago
> sr/a=sspitzer

he gave the sr/a= over aim.

Comment 35

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

fyi, message compose uses a different enabling style to html composer.
Attachment #136999 - Flags: review?(neil.parkwaycc.co.uk) → review+
(Assignee)

Updated

15 years ago
Assignee: bz-vacation → timeless
(Assignee)

Comment 36

15 years ago
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.
Status: NEW → RESOLVED
Last Resolved: 15 years ago
Resolution: --- → FIXED

Comment 37

15 years ago
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.
Status: RESOLVED → VERIFIED
Product: MailNews → Core

Comment 38

14 years ago
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.

(Assignee)

Updated

13 years ago
Blocks: 277715
Product: Core → MailNews Core
You need to log in before you can comment on or make changes to this bug.