1665174 - Smilie insertion causes weird behavior of ENTER key

Status: VERIFIED FIXED

(Thunderbird :: Message Compose Window, defect)

Description

[Jim Garrison]

User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:80.0) Gecko/20100101 Firefox/80.0

Steps to reproduce:

TB 78.2.2

1. Open message compose in HTML mode
2. Insert a smilie using the menu just above the composition area
3. Press ENTER
4. Press ENTER

Actual results:

After step 3 the blinking cursor disappears but nothing else happens
After step 4, a new line followed by a second newline and an extra copy of the smilie appears. The blinking cursor appears above the second smilie.

Also, selection highlighting and cursor movement seem to be thoroughly confused by smilies. For example, pressing the END key at the beginning of a line that ends with a smilie does not move the blinking cursor to the end of the line. Also is is not possible to select a smilie with the mouse.

Expected results:

Seems like HTML composition with smilies needs more testing. It appears to be rather broken and exhibits unexpected behavior.

Comment 1

[Magnus Melin [:mkmelin]]

I can't confirm exactly that, but there's some odd things. If the first thing to insert in a new window is a smiley, then the mode changes from Paragraph to Body. Likely some of the other odd behaviour can be blamed on that.

Comment 2

[Khushil Mistry [:khushil324]]

Attachment: Bug-1665174_smiley-insertion-weird-behavior-0.patch

Comment 3

[Khushil Mistry [:khushil324]]

Attachment: Bug-1665174_smiley-insertion-weird-behavior-1.patch

Comment 4

[Magnus Melin [:mkmelin]]

Comment on attachment 9178412 [details] [diff] [review]
Bug-1665174_smiley-insertion-weird-behavior-1.patch

Review of attachment 9178412 [details] [diff] [review]:
-----------------------------------------------------------------

::: mail/components/compose/content/ComposerCommands.js
@@ +1699,4 @@
>        editor.insertElementAtSelection(smileyElement, true);
> +      // Persist the paragraph state if it gets changed by the controller
> +      // after the insertion of the smiley.
> +      goDoCommandParams("cmd_paragraphState", state);

This kind of works at least for some cases, though not convinced it works on a general level (e.g. adding smileys inside the variety of other elements.

Probably (didn't check) we should just use insertHTML instead of insertElementAtSelection. That way the superflous span is also eliminated.

Comment 5

[Khushil Mistry [:khushil324]]

Attachment: Bug-1665174_smiley-insertion-weird-behavior-2.patch

Comment 6

[Magnus Melin [:mkmelin]]

Comment on attachment 9178687 [details] [diff] [review]
Bug-1665174_smiley-insertion-weird-behavior-2.patch

Review of attachment 9178687 [details] [diff] [review]:
-----------------------------------------------------------------

LGTM, r=mkmelin
Will update the commit msg a bit

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/671f42e78d99
for inserting smileys just insert the smiley char, without any span around it. r=mkmelin DONTBUILD

Comment 8

[p≡p Project]

Bug 1518477 and friends? Like bug 1198292 and bug 1335871 (taken from from https://mzl.la/2Zxbv91).

Comment 9

[Magnus Melin [:mkmelin]]

Probably many of those can be closed. After this patch, it's just one character really so the possibility of problems has been dramatically reduced. If there are problems those probably are in the underlying editor and not on Thunderbird.

Comment 10

[Khushil Mistry [:khushil324]]

Comment on attachment 9178687 [details] [diff] [review]
Bug-1665174_smiley-insertion-weird-behavior-2.patch

[Approval Request Comment]
Regression caused by (bug #): bug 1638874
User impact if declined: Smiley usage in Message compose window will be affected.
Testing completed (on c-c, etc.):
Risk to taking this patch (and alternatives if risky): Low

Comment 11

[Wayne Mery (:wsmwk)]

Comment on attachment 9178687 [details] [diff] [review]
Bug-1665174_smiley-insertion-weird-behavior-2.patch

[Triage Comment]
Approved for beta

Comment 12

[Rob Lemley [:rjl]]

Thunderbird 82.0b3:
https://hg.mozilla.org/releases/comm-beta/rev/faa3265fa49c

Comment 13

[WaltS48 [:walts48]]

Looks good to me testing the 82.0b3 release candidate on Ubuntu 18.04 LTS.

Comment 14

[Wayne Mery (:wsmwk)]

Comment on attachment 9178687 [details] [diff] [review]
Bug-1665174_smiley-insertion-weird-behavior-2.patch

[Triage Comment]
Approved for esr78

Comment 15

[Rob Lemley [:rjl]]

This has a dependency on another bug it looks like.

Comment 16

[Khushil Mistry [:khushil324]]

There is some confusion, I don't see updated smiley usage(inserting Unicode characters instead of images) in 78.4.0

Comment 17

[Khushil Mistry [:khushil324]]

We missed an uplift for bug 1638874. I have combined both the patches. How should we proceed now?

Comment 18

[Khushil Mistry [:khushil324]]

Attachment: Bug-1638874_improve-emoji-support-ESR78.patch

Comment 19

[Rob Lemley [:rjl]]

(In reply to Khushil Mistry [:khushil324] from comment #17)

We missed an uplift for bug 1638874. I have combined both the patches. How should we proceed now?

Interesting... bug 1638874 was pushed to comm-central on 2020-06-15 per bug 1638874 comment 17, meaning it landed in milestone 79.

Thanks for tracking this down, I can just uplift bug 1638874 along with this one.

Comment 20

[Rob Lemley [:rjl]]

Thunderbird 78.4.0:
https://hg.mozilla.org/releases/comm-esr78/rev/4009f753bcfc

Comment 21

[Vlad Lucaci, Thunderbird QA (:vlucaci)]

Confirming this issue as verified fixed on macOS 14, Ubuntu 24 and Windows 11 using ESR 128.0(20240703144221).