182941 - Can't type space after smiley face in html compose

Status: RESOLVED FIXED

(MailNews Core :: Composition, defect)

Description

[Todd]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.3a) Gecko/20021123
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.3a) Gecko/20021123

Hi All,

    I am using 1.3.0.2002112304 on w2k-p, sp3.

    If seems that when I am composing a new message in html and
insert a (pull down) smiley face, I can type any character after it,
except a space.  

     Workaround:  type four spaces, then arrow back two spaces, insert the
smiley face, then arrow forward two spaces and continue typing.

Many thanks,
--Tony
[e-mail]

Reproducible: Always

Steps to Reproduce:
1.
2.
3.

Comment 1

[R.K.Aa.]

related: bug 118912

Comment 2

[Todd]

Hi All,

   I just tried this out in 1.4rc1 on w2k-p, sp3.  I can type anything
after the emoticon picture, except a space.  What a pain in the neck!

    Idea:  I noticed that if you email a list of the emoticons to
yourself, that they show up with a space after the picture.  Hmmm.  Maybe
this has something to do with why I can not type a space after an emoticon
in compose.  ":-Ptounge out" shows in recieve mail as ":-P tounge out"

--Tony

Comment 3

[Mike Cowperthwaite]

Still in RC2, as well:
  Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.4) Gecko/20030612

If I compose an HTML mail as follows (including the brackets, @=smiley):
]@[
]@  [
]@@[
the HTML that gets generated is:
]<span class="moz-smiley-s1"><span> :-) </span></span>[<br>
]<span class="moz-smiley-s1"><span> :-)&nbsp;&nbsp; </span></span>[<br>
]<span class="moz-smiley-s1"><span> :-) </span></span><span 
class="moz-smiley-s2"><span> :-( </span></span>[<br>

When rendered as HTML, the leading space is displayed prior to the smiley 
graphic, but no trailing space(s).  Since the generated HTML includes a leading 
and a trailing space by default, even before the user types anything else, the 
failure to render the spaces seems wrong; the nested <span> seems superfluous at 
best; and including a typed space within the span (unlike any other typed 
character) seems wrong.

If the mail is sent as plain text, the user's typed spaces as well as the 
trailing space generated automatically are included in the mail.

Comment 4

[Dean Winters]

when i try to insert a smile face in a plain text email, i cant type a space
after it, i have to start typing the text directly following the smile face. 
this problem is happening to me in 1.7a.

Comment 5

[Mike Cowperthwaite]

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

Comment 6

[Mike Cowperthwaite]

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

Comment 7

[Mike Cowperthwaite]

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

Comment 8

[Mike Cowperthwaite]

From the dupe:
> After repeating these steps several time, the smiley could not be deleted
> anymore and when I hit the [BACK SPACE] key the characters to the left of
> the smiley were deleted. After all characters to the left of the smiley were
> deleted I could positino the curser before the smiley and delete it with the
> [DEL] key.

I was unable to reproduce this.


Two other possibly related smiley bugs:  xref bug 245632, bug 237852.

Comment 9

[Michèle]

I'm using XP fully updated (for what it's worth). Same problem.

+ Not possible to use arrows or spacebar to pass them.

Cursor stops before (or after - depending on where it was) the smiley. It is
impossible to add blank space before or after + it's impossible to cross/pass
using arrows.

The only way I can add a blank space, is by writing a letter, adding the number
of spaces, that I want AND THEN deleting the first "fiktious" letter again.

Or type the 4 blank spaces mentionned below.

To pass a smiley, I need to use 'home' or 'end' and then navigate from there
using CTRL + arrow r/l, until I reach the point I want to edit.

Would be very nice to be able to pass them using CTRL + arrows. :-)

Kind regards Michèle Vandborg

Comment 10

[Mike Cowperthwaite]

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

Comment 11

[Graeme Bunyan]

(In reply to comment #9)
> I'm using XP fully updated (for what it's worth). Same problem.
> 
> + Not possible to use arrows or spacebar to pass them.
> 
> Cursor stops before (or after - depending on where it was) the smiley. It is
> impossible to add blank space before or after + it's impossible to cross/pass
> using arrows.
> 
> The only way I can add a blank space, is by writing a letter, adding the 
number
> of spaces, that I want AND THEN deleting the first "fiktious" letter again.
> 
> Or type the 4 blank spaces mentionned below.
> 
> To pass a smiley, I need to use 'home' or 'end' and then navigate from there
> using CTRL + arrow r/l, until I reach the point I want to edit.
> 
> Would be very nice to be able to pass them using CTRL + arrows. :-)
> 
> Kind regards Michèle Vandborg

Hi,

I'm seeing the same thing with Thunderbird 1.0.6 (20050716) on Redhat Enterprise 
3...

Thanks,
Graeme.

Comment 12

[Parkhideh]

A confirmation of comment #8 and a reproducible method:
1) type two lines of text.
2) insert an emoticon in the middle of line one.
3) press space bar (and nothing happens).
4) press right cursor key once.
5) press left cursor key once.  Now the cursor is on the right of emoticon.
6) press BackSpace.  Instead of the emoticon, the character before it will disappear.
This problem also exists in Netscape 7.1 Mail.

While doing above sequence my cursor blinker disappeared, but I can not reproduce that one. :)

Comment 13

[Arivald]

I check it, and found why it is working bad:

after inserting smile, code looks:

<span class="moz-smiley-s1"><span> :-) </span></span>

but pressing space, adds &nbsp; inside inner <span>:

<span class="moz-smiley-s1"><span> :-) &nbsp;&nbsp;&nbsp;</span></span>


And css for smile looks:

span.moz-smiley-s16 > span
{
    display: none;
}

Whole inner span is hidden.


I think bug in this case is placing &nbsp; inside span, instead of outside.
I found if there is no space after ":-)" then editor works correctly:

<span class="moz-smiley-s1"><span>:-)</span></span>


What is need to be fixed in file "ComposerCommands.js":
in object

var nsSetSmiley =
{
[...]

      var intElement = editor.createElementWithDefaults("span");
      if (!intElement)
        return;

/* remove / comment out this code block
      //just for mailnews, because of the way it removes HTML
      var smileButMenu = document.getElementById('smileButtonMenu');
      if (smileButMenu.getAttribute("padwithspace"))
         smileyCode = " " + smileyCode + " ";
*/
      var txtElement =  editor.document.createTextNode(smileyCode);
      if (!txtElement)
        return;
[...]
} 

Comment 14

[Magnus Melin [:mkmelin]]

Arivald: wanna submit a patch with that suggestion?

Comment 16

[html-js]

(In reply to comment #14)
> Arivald: wanna submit a patch with that suggestion?

Seems as he/she wanted to submit a patch, yes

Comment 19

[Stefan]

I don't want to be unfriendly, but it has been over 7 (!!) years now since the bug was first reported! I know that there were more important things to fix in early times of Thunderbird, but now we will soon reach version 3.1 and this annoying bug is still there! Please, I can't solve it since my programming skills are too poor to make this, but I bet it's just a few lines of code for a skilled one...

Please fix it!

Comment 20

[Todd]

(In reply to comment #19)
> I don't want to be unfriendly, but it has been over 7 (!!) years now since the
> bug was first reported! I know that there were more important things to fix in
> early times of Thunderbird, but now we will soon reach version 3.1 and this
> annoying bug is still there! Please, I can't solve it since my programming
> skills are too poor to make this, but I bet it's just a few lines of code for a
> skilled one...
> 
> Please fix it!

I would concur with Stefan's sentiments.  Not to be unfriendly either, but seven years is enough.  Please fix this!

Many thanks,
-T

Comment 21

[Belius]

I hope this does not come off as rude but it's been almost 9 (in words: nine!) years and this issue has still not been addressed. Over the years I've introduced a great many people to the world of Thunderbird and almost every time they spotted this issue right away. Seeing how millions of people are using emoticons on an everyday basis and are likely to experience this bug, this is bordering on embarrassing. 

We are in version 5 now -- please somebody have mercy and fix this!

Comment 22

[Ed]

And SeaMonkey is in 2.2 almost ready for 2.3 and still not working here either.

Comment 23

[Todd]

> 2002-12-01 19:18:49 PST 

Nine years dudes!  I am the original poster.  I have Thunderbird spread over two counties.  I have to explain to each person I install Thunderbird on about this bug.  At some point you have to start thinking the developers are being disrespectful of those of us that push their product.  Time to fix this!

-T

Comment 25

[Belius]

Oh well, 2013 has almost passed and Thunderbird version 24 has arrived. But still no solution so far. After almost 12 years this incredibly annoying and glaringly obvious bug MUST go! Someone, anyone have a look at this, please!

Comment 27

[Michal Stanke (Mozilla.cz) [:mstanke][:MikkCZ]]

Still NEW (after more than 11 years). Any chance this bug will be fixed?

Comment 28

[Brian S. Craigie]

I would love to see this fixed.  I couldn't find the ComposerCommands.js file on my PC - presumably it is compiled in somewhere.  It looks like a 5 minute fix.  Can't anyone take this on?  Please? :-)

Comment 29

[Thomas D. (:thomas8)]

Attachment: Patch: remove padwithspace from smileyCode (per comment 13)

The fix outlined in comment 13 seems to work and also improves things somewhat wrt deleting. I don't think padwithspace was still serving any purpose because the padding occurs in the plaintext of the inner <span> which is display:none anyway...

Jörg, could you please put this into the right product/component and choose appropriate reviewer? Tia.

Comment 30

[Jorg K (CEST = GMT+2)]

Comment on attachment 8654501 [details] [diff] [review]
Patch: remove padwithspace from smileyCode (per comment 13)

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

editor/ui reviewers are Neil and Ian.

::: editor/ui/composer/content/ComposerCommands.js
@@ -3028,5 @@
>  
> -      //just for mailnews, because of the way it removes HTML
> -      var smileButMenu = document.getElementById('smileButtonMenu');
> -      if (smileButMenu.getAttribute("padwithspace"))
> -         smileyCode = " " + smileyCode + " ";

This doesn't comply with the coding standard:
1) Capital letter at the start.
2) Full stop at the end.
3) Space after //
4) Wrong indentation after the if
5) if block needs to be enclosed in { }
6) What is the comment trying to say?

Comment 31

[Jorg K (CEST = GMT+2)]

Damn, I see, you're removing this code. OK, then the infringements don't matter.
Reviewers are still Neil or Ian ;-)

Comment 32

[Thomas D. (:thomas8)]

(In reply to Jorg K (GMT+2) from comment #31)
> Damn, I see, you're removing this code. OK, then the infringements don't
> matter.
> Reviewers are still Neil or Ian ;-)

Thanks! I can't believe you really thought a *purist* like me could mess it up like that ;)

Comment 33

[Thomas D. (:thomas8)]

Comment on attachment 8654501 [details] [diff] [review]
Patch: remove padwithspace from smileyCode (per comment 13)

Hi Iann, Jorg suggested in comment 31 you could review this one-line patch.

Smileys work much better without having the padding spaces around the hidden smiley plaintext, and the removed code does not seem to have any useful effect or purpose these days, because the padding occurs in the plaintext of the inner <span> which is display:none anyway (per comment 13)...

Comment 34

[Thomas D. (:thomas8)]

Attachment: Patch v. 1.1: remove padwithspace from smileyCode (per comment 13)

> Hi Ian, Jorg suggested in comment 31 you could review this one-line patch.
> 
> Smileys work much better without having the padding spaces around the hidden
> smiley plaintext, and the removed code does not seem to have any useful
> effect or purpose these days, because the padding occurs in the plaintext of
> the inner <span> which is display:none anyway (per comment 13)...

Comment 35

[Todd]

how soon (what release) will this appear?

Comment 36

[Thomas D. (:thomas8)]

(In reply to Todd from comment #35)
> how soon (what release) will this appear?

In the regular scheme of things, the next official release is 8 March 2016 (6 months from now).
If my patch is accepted, we can try and request to land this earlier in the stability updates every 6 months, so the nearest possible would be 38.3 on 22 September 2015 (which is unlikely).

For details on release dates, see:
https://de.wikipedia.org/wiki/Mozilla_Thunderbird#Versionstabelle

My apologies to everyone for the inexplicable delay and the continuous annoyance this has caused (albeit minor in the big scheme of things). Please keep in mind that we're a small volunteer team maintaining TB and there are thousands of bugs around (10,000+ in TB and Mailnews Core, including unconfirmed). So it's a needle in the haystack/manpower/priorities thing... I've poked into some 3000+ of those bugs over time, but until yesterday I wasn't aware of this particular bug...

Comment 37

[Thomas D. (:thomas8)]

(In reply to Thomas D. from comment #36)
> If my patch is accepted, we can try and request to land this earlier in the
> stability updates every 6 months

Typo: that's every 6 weeks, of course...

> so the nearest possible would be 38.3 on 22 September 2015 (which is unlikely).

Comment 38

[Magnus Melin [:mkmelin]]

Did you check the html and the plaintext it turns into? I think I tried something similar to your patch once, but things didn't really work out.

Comment 39

[Thomas D. (:thomas8)]

(In reply to Magnus Melin from comment #38)
> Did you check the html and the plaintext it turns into? I think I tried
> something similar to your patch once, but things didn't really work out.

Works perfectly on both, because with this patch both leading and trailing spaces are correctly placed outside the special spans of the smiley.

HTML:
> asdf <span class="moz-smiley-s1"><span>:-)</span></span> asdf

Plaintext (downconverted from format auto-detect):
> asdf :-) asdf

Comment 40

[Arivald]

BTW, do You know that this "smileys" aren't shown in any other mail app?

User who insert this nice smileys may expect them to be rendered correctly for the receiver.

I wonder why TB does not include proper style sheets in mails, to make this and other features (signatures, blockquote, ...) visible in any HTML mail app.

Comment 41

[Thomas D. (:thomas8)]

(In reply to Arivald from comment #40)
> BTW, do You know that this "smileys" aren't shown in any other mail app?
> User who insert this nice smileys may expect them to be rendered correctly
> for the receiver.
> I wonder why TB does not include proper style sheets in mails, to make this
> and other features (signatures, blockquote, ...) visible in any HTML mail
> app.

+1. Arivald, I was just thinking exactly the same. It's violating ux-wysiwyg that we show nice smileys which will most likely NOT appear the same way for receiver. As a matter of fact we're sending them as plaintext with a moz-class markup only known only to Thunderbird and Seamonkey so they should just be shown as plaintext when composing. Worse users will never notice because TB converts their received messages, too, and displays nice smileys where there are plain smileys in the source (at least when they have the moz-markup, didn't test otherwise).

I guess it's another relict from plaintext times where things like stylesheets and multipart messages were still alien animals, and increasing the message size with decorations was a taboo...
Unfortunately style sheets and such haven't arrived yet with Thunderbird (bugs filed).

But let's have that discussion elsewhere so that we can fix this bug without distraction.

Comment 42

[Ian Neal]

If you're removing this code, shouldn't the xul attributes be removed too?
http://mxr.mozilla.org/comm-central/search?string=padwithspace&find=\.xul&findi=&filter=^%5B^\0%5D*%24&hitlimit=&tree=comm-central

Comment 43

[Thomas D. (:thomas8)]

(In reply to Ian Neal from comment #42)
> If you're removing this code, shouldn't the xul attributes be removed too?
> http://mxr.mozilla.org/comm-central/search?string=padwithspace&find=\.
> xul&findi=&filter=^%5B^\0%5D*%24&hitlimit=&tree=comm-central

Yes.

Comment 44

[Thomas D. (:thomas8)]

Attachment: Patch v. 1.2: remove padwithspace from smileyCode and XUL

Hi Ian,

a one-minute review of 5-liner patch, as you requested, to remove useless padwithspace attribute from smiley in XUL and js. The attribute did nothing useful but prevented users from typing spaces after smileys.

Comment 45

[aleth [:aleth]]

https://hg.mozilla.org/comm-central/rev/1ec5958134965cf1e346e0f405c76015a0631699
Bug 182941 - Remove useless padwithspace="true" attribute for smileys to fix space-editing problems. ui-r=ThomasD, r=IanN