Ctrl + Backspace and Ctrl + del does not behave correctly (Windows)

RESOLVED FIXED in mozilla1.1beta

Status

SeaMonkey
Composer
RESOLVED FIXED
16 years ago
6 years ago

People

(Reporter: Jan Ehlers, Assigned: Kathleen Brade)

Tracking

({helpwanted, pp})

Trunk
mozilla1.1beta
x86
Windows 2000
helpwanted, pp

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments, 3 obsolete attachments)

(Reporter)

Description

16 years ago
On Windows Ctrl + Backspace normally deletes the word to the left of the cursor,
in Mozilla it does not do anything. 

On Windows Ctrl + Delete normally deletes the word to the right of the curser -
in Mozilla it deletes to the end of the line. 

These problems are both present when editing text in forms and in mails.
(Assignee)

Comment 1

16 years ago
-->akkana
Assignee: kin → akkana

Comment 2

16 years ago
We currently have bindings on Windows to delete to end of line on this key,
which conflicts with aaron's keyboard spec.  mkaply added these bindings,
apparently for bug 79667.  Previously it did cmd_copyOrDelete (which also isn't
consistent with Aaron's spec).

Reading bug 79667, it sounds like maybe someone misunderstood the ctrl-del
binding and thought it should delete to end rather than delete forward word.
Assignee: akkana → mkaply
Component: Editor: Core → Keyboard Navigation

Comment 3

16 years ago
Normally are vrey relative words here.

In a windows Entryfield, Ctrl+Delete deletes to the end of the line, and 
Ctrl+Backspace does delete the word to the left.

In Notepad, Ctrl+Delete deletes to the end of the line.

I believe that Ctrl+Delete is correctly deleting to the end of the line as per 
typical windows applications.
(Reporter)

Comment 4

16 years ago
You are right - normally is relative - it only refers to programs meant for
editing text. 
As I see it Notepad, and windows entryfields are not meant for doing much editing. 

Ctrl + backspace and ctrl + delete behave the way I have described in most
popular programs meant for editing text. This includes Wordprocessors like
Microsoft Word (+ rest of the MS Office Suite), editors like Ultra-edit,
Wordpad, Mail programs like outlook / outlook express, and Eudora. Internet
Explorer behaves in this way too. Pegasus mail and Opera has the ctrl +
backspace functionality that I described, but not the ctrl + delete.
Judging from this, my guess is that many users will expect this behavior, when
editing text.

Comment 5

16 years ago
So maybe we should have different keystrokes for editor than for entry fields?
(Reporter)

Comment 6

16 years ago
Well, I don't really think so we should have variable behavior - that would be
most inconsistent. So go for the best and most flexible behavior everywhere -
that is, the behavior I was suggesting.

Comment 7

16 years ago
It's not variable, it's the same as the OS.

In things like entryfields, Mozilla should be have like the OS. So on 
Windows, Ctrl+Delete should delete to the end of line.

People shouldn't get different behaviors just because they are using Mozilla.

If you are using word, even though Ctrl+Delete deletes a word in the editor, 
Ctrl+Del still deletes to the end of line in any entryfield in Word.

The goal is consistency with the operating system.

Comment 8

16 years ago
I think I accidentally assigned this to myself.
Assignee: mkaply → akkana

Comment 9

16 years ago
-> Aaron for decision on the proper Windows behavior.
Assignee: akkana → aaronl

Comment 10

16 years ago
Well I see both Jan and Mike's point.

I never even realized there were places that Ctrl+Delete didn't delete from the
cursor to the end of the word, and deleted to the end of the line instead.

Personally that seems very strange. It isn't consistent, even in Microsoft apps.
I loaded up Word 2000 and tried it in the style name entrey field on my tool bar
- deletes to end of the word. Therefore, we can see that even in some Microsoft
apps they strove for consistency.

Since people have asked my opinion I'll give it, but I could live with either
solution.

IMHO, Ctrl+Delete has always been known to delete to the end of the word. I
would say that having it sometimes do one and sometimes the other is bad design.
It's also potentially destructive to delete more than what the user may have
intended. Perhaps we should just follow Microsoft Word here, and have
Ctrl+Delete always delete to the end of the word. That way we are more
consistent, traditional and less potentially destructive.

Comment 11

16 years ago
if you want to delete to end of word, use ctrl-right,delete. if you want to 
delete to end of line, use ctrl-delete.

Comment 12

16 years ago
The owner of the keyboard spec says:

Ctrl+delete to delete to end of word
Ctrl+backspace to delete to beginning of word

See my comment #10 for justification
Assignee: aaronl → akkana

Comment 13

16 years ago
Okay, I can do that.  Accepting, attaching patch, looking for review/sr.
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Whiteboard: Needs review, sr
Target Milestone: --- → mozilla0.9.8

Comment 14

16 years ago
Created attachment 61671 [details] [diff] [review]
Change the windows bindings to deleteWordForward

I don't understand why we have editing bindings on the browser window, but I
changed the binding there as well as in the three editor places.
(Assignee)

Comment 15

16 years ago
r=brade
Keywords: pp
Whiteboard: Needs review, sr → Needs sr
*** Bug 116985 has been marked as a duplicate of this bug. ***

Comment 17

16 years ago
Comment on attachment 61671 [details] [diff] [review]
Change the windows bindings to deleteWordForward

r=cmanske
(brade reviewed this as well)
Attachment #61671 - Flags: review+

Comment 18

16 years ago
In most windows controls I use

ctrl+shift+del = delete to end of line
ctrl+del = delete to end of word

I think notepad is backwards compatible with windows 3.1 so they've opted for
old style controls

Comment 19

16 years ago
akk, who should super-review here? Is this something you still want to get in
for 0.9.8?

Comment 20

16 years ago
I was under the impression that keybindings would be discussed instead of 
simply selected by the owner. Comment 10 is invalid. it uses the word 'always' 
which is clearly wrong.

Less destructive is a red herring, ctrl-z should undo whatever crtl-delete 
does.

asa: if you truly care you would be aware that any random sr= can provide the 
stamp, it's a trivial change.

Comment 21

16 years ago
Kin gave his sr pending discussion with timeless; timeless said okay though he
still disagrees with the binding.  Fix is in.
Status: ASSIGNED → RESOLVED
Last Resolved: 16 years ago
Resolution: --- → FIXED
Whiteboard: Needs sr

Comment 22

16 years ago
Current behaviour in Build 2002011503:

CTRL+DELETE deletes to end of word
CTRL+BACKSPACE does not do a thing.

OS: Win98 SE


=> Bug not fixed.

As to the behaviour of CTRL+DELETE, take the following string

aaaa bbbbbbbbb ccccc|eeeeeee dddddddddddddd

| is the cursor ccccceeeeeee is one word.

CTRL+DELETE does the following:

aaaa bbbbbbbbb ccccc|dddddddddddddd
(| is the cursor again).

Is it correct that there is no space between the c-word and the d-word?

Should the bug be reopened,  or is it fixed but just not in the build yet? (01-15).

Comment 23

16 years ago
The Ctrl+delete behavior you describe is correct.

However, ctrl+backspace should work as well.

Comment 24

16 years ago
Does not work in Win98 in build 2002032803.

See also bug #128397
Status: RESOLVED → REOPENED
Resolution: FIXED → ---

Comment 25

16 years ago
Only CTRL+Backspace is affected. CTRL+Delete works. 

Sorry, should have checked this.

Comment 26

16 years ago
*** Bug 128397 has been marked as a duplicate of this bug. ***

Comment 27

16 years ago
I don't understand why ctrl-backspace isn't deleting backward.  In
platformHTMLBindings I see VK_BACK bound to cmd_deleteWordBackward for
inputFields, textAreas, and editor.

Need to pass this off to someone who has windows for debugging why this isn't
working.  Aaron?
Assignee: akkana → aaronl
Status: REOPENED → NEW

Comment 28

16 years ago
*** Bug 136626 has been marked as a duplicate of this bug. ***

Comment 29

16 years ago
Resetting milestone, assigning to composer team.
Assignee: aaronl → syd
Component: Keyboard Navigation → Editor: Composer
Target Milestone: mozilla0.9.8 → ---
(Assignee)

Comment 30

16 years ago
I'll take this bug; I suspect the problem relates to bug 50255 (if it works
correctly on Linux).  
Assignee: syd → brade
Keywords: helpwanted
Target Milestone: --- → mozilla1.2alpha

Comment 31

16 years ago
Yes, ctrl-backspace is working on linux, so you're probably right that it's a
problem with windows key events.  Should it be a dup?
(Assignee)

Comment 32

16 years ago
Bernie, Dean--can one of you help with this bug?  I think it's similar to bug
50255.  :-/

Comment 33

16 years ago
Created attachment 79501 [details] [diff] [review]
Fix to pass the keypress event for <ctrl>+<backspace>

Brade, yes you are correct, the ctrl+backspace keycombination was being
inproperly processed in the onchar handler as a char code 127 (not 08). 

So here is a patch to process the keydown event for ctrl+backspace in the
keydown handler (where it is a code=08). I know this is another 'special case',
but without rewriting both the onchar and onkeydown handlers, 'special casing'
seem to be the only way. 

Dean, do you have the 'better test case' attachment 55849 [details] to bug 50255? It
seems to have disappeard from 50255 if you try to look at it you just get a
blank display. Please email it to me (bernie5412@hotmail.com) or repost if
possible, thanks.

Comment 34

16 years ago
Created attachment 79509 [details] [diff] [review]
Repost 79501patch to fix formatting problem

Reposting previous patch to fix formatting problem. Sorry..

Comment 35

16 years ago
Bernie, the test case works fine.  It starts as a blank page, until you press a key.

Comment 36

16 years ago
Dean, 
My problem is that I don't have the source on my computer any more and if I try 
to open the attachment on bug 50255 it's empty, so I can't get another copy. Am 
I doing something wrong?

Comment 37

16 years ago
Dean, I figured it out. I have to right click on the attachment link in 50255 
amd 'save target as' or in the edit window (which I was seeing as blank) I can 
right click and view source and then save it locally. I guess I forgot that I 
did this once before. 
 

Comment 38

16 years ago
Hrm.  View Source doesn't work when looking at the attachment but Save Page does.

Comment 39

16 years ago
Comment on attachment 79509 [details] [diff] [review]
Repost 79501patch to fix formatting problem

Glad to see you're using my MapVirtualKey() suggestion.  I have much more
confidence in this.

r=me
Attachment #79509 - Flags: review+

Comment 40

16 years ago
Comment on attachment 79509 [details] [diff] [review]
Repost 79501patch to fix formatting problem

>Index: src/windows/nsWindow.cpp
>===================================================================
>RCS file: /cvsroot/mozilla/widget/src/windows/nsWindow.cpp,v
>retrieving revision 3.411
>diff -u -r3.411 nsWindow.cpp
>--- src/windows/nsWindow.cpp	10 Apr 2002 04:31:23 -0000	3.411
>+++ src/windows/nsWindow.cpp	16 Apr 2002 21:32:25 -0000
>@@ -2702,6 +2702,9 @@
>   else if (mIsControlDown && aVirtualKeyCode == NS_VK_TAB) {
>     DispatchKeyEvent(NS_KEY_PRESS, 0, NS_VK_TAB, aKeyData);
>   }
>+  else if (mIsControlDown && aVirtualKeyCode == VK_BACK) {
>+    DispatchKeyEvent(NS_KEY_PRESS, 0, aVirtualKeyCode, aKeyData);
>+  }

How about passing VK_BACK to be consistent with the preceding code.


>+            // <ctrl><backSpace> also generates a char code event but it is handled in 
>+            // the KeyDown method, so we can ignore it here as we do for ctrl+enter.
>+            if ((mIsControlDown && virtualKeyCode == VK_RETURN)  ||
>+							  (mIsControlDown && virtualKeyCode == VK_BACK)) {

Use spaces, not hard tabs. Hoist `mIsControlDown' condition.

Comment 41

16 years ago
kmcclusk or roc should probably review the changes to nsWindow.cpp (attachment
79509 [details] [diff] [review]). kin should probably super-review the key binding changes (attachment 61671 [details] [diff] [review]).

Comment 42

16 years ago
Created attachment 81925 [details] [diff] [review]
Revised patch for nsWindows.cpp re C. Waterson's comments

Thanks Chris for the comments. Changes made in this revision. 

Can you r= or should I ask kmcclusk or roc for r= and you for sr= ?
Rod is the owner of nsWindow.cpp. Rod: Can you review this patch?
Comment on attachment 61671 [details] [diff] [review]
Change the windows bindings to deleteWordForward

sr=jst
Attachment #61671 - Flags: superreview+

Comment 45

16 years ago
If you are no longer using "sSkipWMCHARProcessing" in the "if" statement then it
should be removed altogether. Have you passed the MapVirtualKey key by Frank or
somebody in the International team? They really need to ok it also.

Comment 46

16 years ago
My apologies for not catching the indentation problem in my original review.

For a little background on how we got to using MapVirtualKey(), see bug 50255
comment 181 and bug 50255 comment 196.

Comment 47

16 years ago
Created attachment 82242 [details] [diff] [review]
Patch to nsWindows.cpp - removed sSkipWMCHARProcessing

Removed sSkipWMCHARProcessing per Rod's comments. 

Sorry, but I'm new, who is "Frank"?
(Assignee)

Comment 48

16 years ago
Frank, Naoki, Roy--can one of you look at the patch for i18n issue perspective?
Summary: Ctrl + Backspace and Ctrl + del does not behave correctly → Ctrl + Backspace and Ctrl + del does not behave correctly (Windows)

Comment 49

16 years ago
IME doesn't process WM_CHAR msg, instead we tell IMEs to send 
WM_IME_COMPOSITION msgs.
So _theoretically_ removing sSkipWMCHARProcessing shouldn't impact
IME msg processing.

I'll test the patch on my JA system; but the patch looks good after
quick scan.

Comment 50

16 years ago
Roy, 

How did your test go on your JA system? Can we proceed to get r= and sr=?

Comment 51

16 years ago
*** Bug 142640 has been marked as a duplicate of this bug. ***

Comment 52

16 years ago
Hi, I wanted to report this but I see it's already under discussion.  In the
interest of adding to the discussion, I saw Chris Watterson's comment #40 with
the following snippet of code:

>+            // <ctrl><backSpace> also generates a char code event but it is
handled in 
>+            // the KeyDown method, so we can ignore it here as we do for
ctrl+enter.
>+            if ((mIsControlDown && virtualKeyCode == VK_RETURN)  ||
>+
						  (mIsControlDown && virtualKeyCode == VK_BACK)) {

The "if" could be grouped better.  Rather than an "||" with two separate "&&"s,
it could be written as a single "&&" with a single "||":

>+            // <ctrl><backSpace> also generates a char code event but it is
handled in 
>+            // the KeyDown method, so we can ignore it here as we do for
ctrl+enter.
>+            if (mIsControlDown && ((virtualKeyCode == VK_RETURN)  ||
>+
	                     (virtualKeyCode == VK_BACK)) {

It's slightly easier to understand this way, as well.

Comment 53

16 years ago
*** Bug 143945 has been marked as a duplicate of this bug. ***
(Assignee)

Comment 54

16 years ago
Bernie--the latest patch is still valid, right?  Is it ok to obsolete the old
patches?

Roy--please test and mark r= or indicate what problems you see
roc--please sr=
Target Milestone: mozilla1.2alpha → mozilla1.1beta

Comment 55

16 years ago
Comment on attachment 82242 [details] [diff] [review]
Patch to nsWindows.cpp - removed sSkipWMCHARProcessing

/r=yokoyama
Tested in W2K-Ja with 
IME ON/OFF.
<ctl>+<Del> and
<ctl>+<bsp> works fine.

I also test
<ctl>+<Enter> and
<Shft>+<Enter> for 0x0a
processing.  They look
ok as well.
Attachment #82242 - Flags: review+

Comment 56

16 years ago
Latest patch (82242) is still valid as far as I know. I haven't pulled any 
source in a while so I dont know if the line numbers will be different. 

I dont think you should obsolete patch number 61671, it fixed a different part 
of the problem. 

You can obsolete #79501, 79509, and 81925. These are replaced by 82242. 

Bernie
(Assignee)

Updated

16 years ago
Attachment #79501 - Attachment is obsolete: true
(Assignee)

Updated

16 years ago
Attachment #79509 - Attachment is obsolete: true
(Assignee)

Updated

16 years ago
Attachment #81925 - Attachment is obsolete: true
Comment on attachment 82242 [details] [diff] [review]
Patch to nsWindows.cpp - removed sSkipWMCHARProcessing

sr=roc+moz
Attachment #82242 - Flags: superreview+
(Assignee)

Comment 58

16 years ago
fix checked in on the trunk; Thanks everyone!
Status: NEW → RESOLVED
Last Resolved: 16 years ago16 years ago
Resolution: --- → FIXED

Comment 59

16 years ago
*** Bug 151168 has been marked as a duplicate of this bug. ***
Product: Browser → Seamonkey
You need to log in before you can comment on or make changes to this bug.