118912 - Can't delete smily/smiley/emoticon

Status: VERIFIED FIXED

(Core :: DOM: Editor, defect, P3)

Description

[Neil Marshall]

I opened a compose window, typed some text, placed a smily face using the
toolbar, and typed some more text.  If I then press backspace numerous times,
the text around the smily gets deleted, but not the smily itself.

Neither backspace nor delete will get rid of it.

I have to first select it with the mouse then press delete or backspace.

Comment 1

[Jean-Francois Ducarroz]

Reassign to editor

Comment 2

[Kathleen :Brade]

If you insert an image, can you delete that?
Should this bug be editorbase?

Comment 3

[Kevin McCluskey (gone)]

Nominating for EDITORBASE.
Reproduced with 2002021403 build on WINXP.

Comment 4

[Kevin McCluskey (gone)]

Also reproduced with todays build 20002022003 build on WinXP.

Comment 5

[Syd Logan]

EDITORBASE-

Comment 6

[Neil Marshall]

If I paste a smily, then hit backspace a couple times, when I send the email all
they get on the other side is ":" instead of ":-)".  It's as if it's deleting
the actual text and not the image of the smily.

Comment 7

[kinmoz]

Yeah that makes sense, I believe [e-mail] implemented the smiley's 
as span wrapping the actual text. The span has style on it that displays the 
image and hides the actual text.

The core editor is unaware of this, and just treats the smiley text as if it 
were in an inline container ... like bold, italic, underline, etc ... so it just 
deletes a char at a time.

Comment 8

[Vadim Berezniker]

*** Bug 140798 has been marked as a duplicate of this bug. ***

Comment 9

[Kathleen :Brade]

seek reconsideration

Comment 10

[Anatoliy Averbukh]

Method described in the bug for delete is incorrect.

Smileys are just background images.
To delete siley, user must:

1. select the smiley by, for instance, mouse click on the smiley
2. push "delete" (or backspace) key 

The smiley will be deleted.

Probably some relnotes needed to be porvided.

Comment 11

[Syd Logan]

images can be deleted by just hitting backspace.

Comment 12

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

Ehehe, that's weirder than we all thought it is.

1. launch Mozilla
2. open (a) a mail composition window (b) Composer
3. in mail compose, type aaaaa, insert a smiley, type bbbbbb
4. select all, copy, paste into Composer, hit return key in Composer
5. position caret right before the first b and hit backspace
6. cycle through steps 4 and 5 at least *6* times
7. in the Composer window, switch to view Source

result in Composer's view source :

  aaaaa<span class="moz-smiley-s1"><span> :-)</span></span>bbbbb<br>
  aaaaa<span class="moz-smiley-s1"><span> :-</span></span>bbbbb<br>
  aaaaa<span class="moz-smiley-s1"><span> :</span></span>bbbbb<br>
  aaaaa<span class="moz-smiley-s1"><span> </span></span>bbbbb<br>
  aaaaa<span class="moz-smiley-s1"><span></span></span>bbbbb<br>
  aaaaabbbbb<br>

So the smiley is not removed by a hit on backspace because we only remove
one character at a time in the corresponding span.

Hope that helps.

Comment 13

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

Btw, Kathy, you added the regression kwd, but as states it Anatolyi in comment #10
it is _not_ a regression. It seems to me that it never worked because there is
no magic at all in the editor for the detection of span[class^="-moz-smiley"]

Comment 14

[Syd Logan]

EDITORBASE- since it doesn't affect the core composer app. Leaving nsbeta1+ for
kevin/jfrancis to prioritize as they see fit

Comment 15

[Kathleen :Brade]

*** Bug 166285 has been marked as a duplicate of this bug. ***

Comment 16

[Kathleen :Brade]

*** Bug 164136 has been marked as a duplicate of this bug. ***

Comment 17

[Joe Francis]

Once # 180034 lands all we need to do is set -moz-user-select: all on the span.
 Then the editor will delete it atomically.

Comment 18

[R.K.Aa.]

*** Bug 182057 has been marked as a duplicate of this bug. ***

Comment 19

[Kathleen :Brade]

*** Bug 189116 has been marked as a duplicate of this bug. ***

Comment 20

[Joe Francis]

Attachment: patch to messageBody.css

Comment 21

[Joe Francis]

Attachment: patch to EditorContent.css

Comment 22

[Joe Francis]

Comment on attachment 116660 [details] [diff] [review]
patch to messageBody.css

not really sure who reviewers should be, but this it pretty trivial regardless

Comment 23

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

Comment on attachment 116660 [details] [diff] [review]
patch to messageBody.css

r=glazman

Comment 24

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

Comment on attachment 116661 [details] [diff] [review]
patch to EditorContent.css

r=glazman

Comment 25

[kinmoz]

Comment on attachment 116661 [details] [diff] [review]
patch to EditorContent.css

sr=[e-mail]

Comment 26

[kinmoz]

Comment on attachment 116660 [details] [diff] [review]
patch to messageBody.css

sr=[e-mail]

Comment 27

[saari (gone)]

EDITORBASE+ topembed+ normalization

Comment 28

[Joe Francis]

fix landed on tip

Comment 29

[rubydoo123]

maybe I jumped the gun on this one, but using the build from 2003032404, using
the backspace did not delete the smiley. I followed Daniel's test. After pasting
the aaaaa(smiley)bbbbb, I placed the caret to the right of the last 'b' and
selected backspace. When I reached the smiley, I hot backspace and the caret
shifted to the left of the smiley permitting me to continue deleting the a's --
but the smailey remained.

Comment 30

[Joe Francis]

not sure why this isn't working.  reopening.

Comment 31

[Kevin McCluskey (gone)]

Kai can you look at this when you get a chance?

Comment 32

[Samir Gehani]

adt: nsbeta1+/adt2

Comment 33

[Kai Engert (:KaiE:)]

I think the patch in this bug has made the situation worse.

I see a regression that is now present on trunk builds, and by backing it out
locally, I confirmed is caused by this patch.

For some reason, the regression is only seen the first time you try it in a
Mozilla session!

To see the regression:
- start Mozilla
- open a mail composer window
- type some chars, like "wfwkfpwef"
- use smiley toolbar, insert topmost smiley
- type any letter, e.g. "x"

Actual behaviour: The smiley gets deleted!

I backed out the
 -moz-user-select: all;
lines in all three files modified by the patches in this bug.

This makes the regression go away, but of course, the original bug reappears.

Could somebody please explain what kind of effect -moz-user-select is supposed
to control?


What approach do you think is more reasonable:
1) back out the patch and find another solution to the original bug?
or
2) keep the -moz-user-select addition and try to figure out why typing a char
deletes the smiley?

Comment 34

[Kai Engert (:KaiE:)]

However, while we see this regression, the original patch seems fixed to me,
hitting backspace deletes the smiley, as opposed to what beepe saw two months
ago (comment 29).

Comment 35

[Kai Engert (:KaiE:)]

> Could somebody please explain what kind of effect -moz-user-select is supposed
> to control?

Ok, I found some information and now understand the meaning. We want to delete
the smiley chars in an atomic operation.


I guess the conclusion is, this bug is fixed, I always can use backspace to
delete a smiley. Marking fixed again. Beth, can you please test again?


I have filed the new bug 207183 to track the regression.

Comment 36

[Joe Francis]

The regression is most likely caused by the selection being inside the span when
the smiley is inserted. 

Probably the code that inserts the smiley places the selection beyond the span,
but then later one (in the processing handled by
nsHTMLEditRules:AfterEditInner()) the selection is getting pulled back into the
span (in the case where the smiley is at the end of it's block, as is often the
case when you insert it as you type).

The selection adjusting code needs to make sure that it does not cross
-moz-user-select: all boundaries when adjusting the selection.

Comment 37

[Kai Engert (:KaiE:)]

> The regression is most likely caused by the selection being inside the span
> when the smiley is inserted. 

Discussed with Joe on IRC, this is not the cause.
The bug still is seen if one moves the caret around in the document, moving to
back to the position after the smiley.

Comment 38

[rubydoo123]

using the trunk build from 20030612 on win2K

1. opened mail
2. created a new message with the following:
aaaa[smiley]bbbb
3. put focus at the end of the line and hit backspace

smiley was deleted as expected

Comment 39

[Serg Iv]

I can't delete smileys with the backspace key in TB17. The delete key works fine.
Steps to reproduce:
1. Write:
aaaaa[smiley]aaaaaaaa
2. Press backspace numerous times

Result: only text deleted

I'm not sure if i should file another bug for my issue.