350564 - Non-standard behaviour on word deletion when a non-collapsed selection exists

Status: RESOLVED FIXED

(Core :: DOM: Editor, defect)

Description

[Simon Montagu :smontagu]

When deleting by word, with ctrl-delete or ctrl-backspace, and there is already an active non-collapsed selection, what should happen?

The standard behaviour varies by platform. On Windows the existing selection is collapsed to start, and then the deletion is performed. On Mac and Linux, the existing selection is deleted (in other words the behaviour is identical for delete, backspace, ctrl-delete and ctrl-backspace).

In Gecko, the existing selection is extended by word boundaries in the direction of the deletion (forwards for delete and backwards for backspace, and then the resulting selection is deleted.

A table may make this clearer. Assume an input field containing "one two three four five" in which "two three four" is selected.

Resulting text in a native Windows rich text control:
                      
After ctrl-delete:      one three four five 

After ctrl-backspace:   two three four five


Resulting text in Gecko:

                     Selecting forwards        Selecting backwards
After ctrl-delete:     one                         one two  five

After ctrl-backspace   one four five               five

Comment 1

[Simon Montagu :smontagu]

Attachment: Patch

At Uri's suggestion, I made the behaviour depend on the existing layout.selection.caret_style pref from bug 299417.

Comment 2

[Daniel Glazman (:glazou) (not active in Mozilla any more)]

Comment on attachment 236073 [details] [diff] [review]
Patch

r=[e-mail]

Comment 3

[[e-mail]]

Comment on attachment 236073 [details] [diff] [review]
Patch

>+    prefBranch->GetIntPref("layout.selection.caret_style", &mCaretStyle);
>+#ifdef XP_WIN
>+    if (mCaretStyle == 0)
>+      mCaretStyle = 1;
>+#endif
Not your fault, but why do we bother having ifdefs in all.js i.e.
#ifdef XP_WIN
pref("layout.selection.caret_style", 1);
#else
#ifdef XP_MACOSX
pref("layout.selection.caret_style", 2);
#else
pref("layout.selection.caret_style", 0);
#endif
#endif
As it is, you've subtly changed the meaning of the preference, and you should update the comment to indicate that the pref affects word delete and if you want to achieve the Unix behaviour (Ctrl+Bksp deletes any selection, but the caret is visible) then you need to set the pref to 3. sr=me with that fixed.

>+      (aAction == eNextWord || aAction == ePreviousWord ||
>+       aAction ==  eToBeginningOfLine || aAction == eToEndOfLine))
Nit: doubled space

>+    if (mCaretStyle == 1)
>+    {
>+      result = selection->CollapseToStart();
>+      if (NS_FAILED(result)) return result;
>+    }
>+    else 
>+      aAction = eNone;
Other code (SplitNodeImpl) seems to like to have lots more {}s.

Comment 4

[Simon Montagu :smontagu]

Checked in.