Shift+Return repeats the last character

[MisterAI]

Description

When entering a new line via Shift+Return when composing a message the last character of the current line is automatically added as the first character of the new line (the cursor is placed before the repeated character).

Steps to reproduce

• Go to "All settings" -> "Preferences" -> Activate "Use Ctrl + Enter to send [...]"
• Start typing a message in any chat (room)
• Hit Shift + Return

Example result:

Test
t

Expected result:

Test
empty line

Log: not sent

Version information

• Platform: desktop

For the desktop app:

• OS: Linux Mint 18.3
• Version:
  • Element version: 1.7.25
  • olm version: 3.2.1

[SimonBrandner]

Looks like this happens only on Chromium (and desktop as it uses it)

[t3chguy]

Index: src/KeyBindingsDefaults.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
--- src/KeyBindingsDefaults.ts	(revision 45acf70b00cb0c51740455d09164b1c3c033b5f3)
+++ src/KeyBindingsDefaults.ts	(date 1620659769892)
@@ -131,6 +131,13 @@
                 key: Key.ENTER,
             },
         });
+        bindings.push({
+            action: MessageComposerAction.NewLine,
+            keyCombo: {
+                key: Key.ENTER,
+                shiftKey: true,
+            },
+        });
     } else {
         bindings.push({
             action: MessageComposerAction.Send,

is a working patch but it kind of feels like the KeyBindingsManager needs a concept of "don't care" for some of ctrl/meta/shift/alt/cmd

[SimonBrandner]

Yep, that would be nice - I wonder if @czeidler has any thoughts on this.

I am also probably missing what is it that exactly happens when you do shift-enter

[czeidler]

Do you know why this is happening? I had a quick look but could figure out why it inserts the last character. Shouldn't the normal text input handle it correctly automatically?

[SimonBrandner]

I've got no idea why it inserts the last character 🤷‍♂️ This appears to be caused by how the composer works? It might make sense to dig into why it is necessary to do model.insert() (or whatever is the name of that method)

[czeidler]

Can confirm that it works as expected in Firefox but not in Chrome.

Ran it through the debugger and it seems like Chrome adds two carriage returns \n\n when at the end of the line and shift + Enter. For example, in Chrome:

1. With an oldValue = 'abc' and caret at position 3 the newValue is 'abc\n\n'.
2. With an oldValue = 'abc' and caret at position 2 the newValue is 'ab\nc'.

Is that expected Chrome behavior or is that a bug? This results breaking the line at the wrong position in the model. Could add a workaround for that if this behavior isn't there for some purpose...

@t3chguy I actually thought about some fuzzy "I don't care" matching but then decided against it. The reason for that was that if the user enters a fuzzy key combo, the user might have done this accidentally. In this case, applying the keybinding would probably be very frustrating (especially when sending the message too early). On the other hand, if the fuzzy key combo was entered on purpose and nothing happens the user simply has to re-enter the correct, exact combo. I argue that the exact matching is less frustrating to the user. Furthermore, it keeps things simple.

[czeidler]

The problem actually happens at every end of line. Below is a fix. However, I have a different solution in a pull request.

@@ -335,6 +335,20 @@ export default class BasicMessageEditor extends React.Component<IProps, IState>
         this.modifiedFlag = true;
         const sel = document.getSelection();
         const {caret, text} = getCaretOffsetAndText(this.editorRef.current, sel);
+        // On Chrome shift+enter generates a double carriage return sequence '\n\n' when:
+        // 1) the previous char is not a \n
+        // 2) the next char is a \n
+        // Also see #17215.
+        // The model gets confused by this unexpected input produces an incorrect line break.
+        // The following block sanitizes the text before passing it to the model.
+        if (event.inputType === 'insertLineBreak' && text.substr(caret.offset - 1, 2) === '\n\n') {
+            const previousValue = this.props.model.parts.reduce((text, p) => text + p.text, "");
+            if (previousValue.length === text.length - 2) {
+                this.props.model.update(text.slice(0, caret.offset) + text.slice(caret.offset + 1),
+                    event.inputType, caret);
+                return;
+            }
+        }
         this.props.model.update(text, event.inputType, caret);
     };

[mcg-matrix]

Example result:

Test
t

Expected result:

Test
empty line

I can report that this unexpected behaviour still occurs in Element-Desktop installed via Debian package "element-desktop" version 1.9.5 provided by https://packages.riot.im/debian/.