Closed Bug 1665174 Opened 1 year ago Closed 1 year ago

Smilie insertion causes weird behavior of ENTER key

Categories

(Thunderbird :: Message Compose Window, defect)

defect

Tracking

(thunderbird_esr78+ fixed, thunderbird82 fixed)

RESOLVED FIXED
83 Branch
Tracking Status
thunderbird_esr78 + fixed
thunderbird82 --- fixed

People

(Reporter: jhg, Assigned: khushil324)

Details

Attachments

(1 file, 3 obsolete files)

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.

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.

Assignee: nobody → khushil324
Attachment #9178411 - Flags: review?(mkmelin+mozilla)
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Attachment #9178411 - Attachment is obsolete: true
Attachment #9178411 - Flags: review?(mkmelin+mozilla)
Attachment #9178412 - Flags: review?(mkmelin+mozilla)
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.
Attachment #9178412 - Flags: review?(mkmelin+mozilla)
Component: Untriaged → Message Compose Window
Attachment #9178412 - Attachment is obsolete: true
Attachment #9178687 - Flags: review?(mkmelin+mozilla)
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
Attachment #9178687 - Flags: review?(mkmelin+mozilla) → review+
Target Milestone: --- → 83 Branch

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

Status: ASSIGNED → RESOLVED
Closed: 1 year ago
Resolution: --- → FIXED

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

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 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

Attachment #9178687 - Flags: approval-comm-esr78?
Attachment #9178687 - Flags: approval-comm-beta?

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

[Triage Comment]
Approved for beta

Attachment #9178687 - Flags: approval-comm-beta? → approval-comm-beta+

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

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

[Triage Comment]
Approved for esr78

Attachment #9178687 - Flags: approval-comm-esr78? → approval-comm-esr78+

This has a dependency on another bug it looks like.

Flags: needinfo?(rob)
Flags: needinfo?(khushil324)

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

Flags: needinfo?(khushil324)

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

Flags: needinfo?(mkmelin+mozilla)
Attachment #9182167 - Attachment is obsolete: true

(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.

Flags: needinfo?(rob)
Flags: needinfo?(mkmelin+mozilla)
You need to log in before you can comment on or make changes to this bug.