Make WordPress Core

Opened 10 years ago

Closed 10 years ago

Last modified 10 years ago

#31242 closed task (blessed) (fixed)

Global Emoji Support

Reported by: pento's profile pento Owned by: pento's profile pento
Milestone: 4.2 Priority: normal
Severity: normal Version:
Component: General Keywords:
Focuses: javascript Cc:

Description

Once #21212 is complete, then WordPress will support storing emoji in the database.

The problem is, some browsers still don't support displaying emoji. We need to work around their lack of useful features.

Here's a plugin to get things rolling:

https://github.com/pento/x1f4a9

Attachments (25)

31242.diff (54.9 KB) - added by pento 10 years ago.
31242.2.diff (62.1 KB) - added by pento 10 years ago.
31242.3.diff (4.3 KB) - added by pento 10 years ago.
31242.patch (7.4 KB) - added by iseulde 10 years ago.
31242.3.patch (7.2 KB) - added by iseulde 10 years ago.
Screen Shot 2015-03-12 at 10.40.58.png (30.0 KB) - added by iseulde 10 years ago.
Screen Shot 2015-03-12 at 10.42.31.png (35.7 KB) - added by iseulde 10 years ago.
31242.2.patch (11.3 KB) - added by iseulde 10 years ago.
31242.4.patch (10.5 KB) - added by iseulde 10 years ago.
31242.5.patch (10.5 KB) - added by iseulde 10 years ago.
31242.6.patch (11.1 KB) - added by iseulde 10 years ago.
31242.7.patch (907 bytes) - added by iseulde 10 years ago.
31242.8.patch (1.1 KB) - added by iseulde 10 years ago.
31242.9.patch (519 bytes) - added by iseulde 10 years ago.
31242.10.patch (929 bytes) - added by iseulde 10 years ago.
31242.11.patch (1.2 KB) - added by iseulde 10 years ago.
31242.12.patch (2.4 KB) - added by azaozz 10 years ago.
31242.13.patch (1.3 KB) - added by iseulde 10 years ago.
31242.14.patch (2.2 KB) - added by iseulde 10 years ago.
31242.15.patch (1.8 KB) - added by iseulde 10 years ago.
31242.16.patch (403 bytes) - added by SergeyBiryukov 10 years ago.
31242.display-block-on-p2.png (9.9 KB) - added by SergeyBiryukov 10 years ago.
31242.17.patch (910 bytes) - added by iseulde 10 years ago.
31242.18.patch (12.0 KB) - added by iseulde 10 years ago.
31242.19.patch (12.8 KB) - added by azaozz 10 years ago.

Download all attachments as: .zip

Change History (93)

#1 @pento
10 years ago

Note to self, for when it comes time to commit this: @batmoo, @joen and @mkaz are owed props for their work on WP.com's emoji support, upon which x1f4a9 is based.

#2 follow-up: @danielbachhuber
10 years ago

We should also update the language of the setting. Emojis are different than emoticons.

Because emojis won't be properly displayed outside of the website context (e.g. search results, RSS feeds, email clients, etc.), should we have some compatibility shim to translate the data on the fly?

https://dl.dropboxusercontent.com/s/0v81fuykb81q6oi/2015-02-06%20at%208.53%20AM%202x.png?dl=0

#3 in reply to: ↑ 2 @pento
10 years ago

Replying to danielbachhuber:

Because emojis won't be properly displayed outside of the website context (e.g. search results, RSS feeds, email clients, etc.), should we have some compatibility shim to translate the data on the fly?

We can definitely do something for RSS, and maybe for email. Not sure about search results, though. I don't know of any way to tell Google (or any search engine) to use an image in search results.

#4 @pento
10 years ago

x1f4a9 is now in the repo, and appears in the Beta Testing tab.

This ticket was mentioned in Slack in #core by pento. View the logs.


10 years ago

#6 follow-ups: @afercia
10 years ago

As per Slack conversation, uploading a screenshot: alt attributes need some love. See in the dev tools, Firefox and Chrome on Windows. Ideally, the alt attributes should be the emoji name, if not possible, better to have an empty alt attribute in lieu of a character that wouldn't be read out by screen readers or read out in a very weird way.

https://cldup.com/3xE2EB7mNb.png

#7 in reply to: ↑ 6 @hnle
10 years ago

Replying to afercia:

Ideally, the alt attributes should be the emoji name, if not possible, better to have an empty alt attribute in lieu of a character that wouldn't be read out by screen readers or read out in a very weird way.

We often drag-select and Copy text&paste.
That time, images will copy as alt-text..

#8 in reply to: ↑ 6 @kraftbj
10 years ago

Replying to afercia:

As per Slack conversation, uploading a screenshot: alt attributes need some love. See in the dev tools, Firefox and Chrome on Windows. Ideally, the alt attributes should be the emoji name, if not possible, better to have an empty alt attribute in lieu of a character that wouldn't be read out by screen readers or read out in a very weird way.

See https://github.com/pento/x1f4a9/issues/6 . I reported it upstream to Twitter (as the issue is in the code we're taking from them) and they wontfix'd it.

#9 @kraftbj
10 years ago

Other issues opened/for discussion:
Emoji not rendered in form fields: https://github.com/pento/x1f4a9/issues/7
Emoji not rendered after ajax actions: https://github.com/pento/x1f4a9/issues/8
A couple of issues with emoji being dropped completely:
text widgets: https://github.com/pento/x1f4a9/issues/9
quick edit: https://github.com/pento/x1f4a9/issues/10
Emoji in terms will display blank slug: https://github.com/pento/x1f4a9/issues/11
Cursor position after TinyMCE emoji render: https://github.com/pento/x1f4a9/issues/12

@pento
10 years ago

#10 @pento
10 years ago

  • Focuses javascript added
  • Keywords has-patch needs-unit-tests added

Well, 31242.diff sure is a thing.

Currently in need of a JS review, and unit tests.

This ticket was mentioned in Slack in #core by pento. View the logs.


10 years ago

This ticket was mentioned in Slack in #core-editor by pento. View the logs.


10 years ago

#13 @afercia
10 years ago

Just noticed something bit weird in IE 8, see screenshot:

https://cldup.com/j8OSbXBGF8.png

See the space below the admin bar. That space disappears as soon as I disable the Emoji plugin. Reproduced in trunk and 4.1 with Twenty Fifteen and just Twitters Emoji plugin active.
@azaozz pointed out that span is the standard MCE bookmark to keep the caret location in IE, I suspect it shouldn't be appended there.

@pento
10 years ago

#14 @pento
10 years ago

  • Keywords commit added; needs-unit-tests removed
  • Owner set to pento
  • Status changed from new to assigned

31242.2.diff fixes up all the smiley unit tests.

I need some sleep, I'll commit this in the morning, along with #31328.

This ticket was mentioned in Slack in #core by drew. View the logs.


10 years ago

#16 @DrewAPicture
10 years ago

  • Type changed from enhancement to task (blessed)

#17 @pento
10 years ago

In 31733:

Add emoji support, with Twemoji fallback.

Replace exisiting smilies with equivalent emoji, or with shiny new smiley images where no emoji existed.

Props batmoo, joen and mkaz for the original plugin upon which this is based.

Props pento, iseulde, kraftbj and peterwilsoncc for making the internet's dreams come true.

See #31242

#18 @nacin
10 years ago

In 31744:

Emoji JS cleanup.

  • Filename: emoji.js => wp-emoji.js
  • Script handle: emoji => wp-emoji
  • Object: WPEmoji => wp.emoji
  • Script settings: EmojiSettings => _wpemojiSettings
  • Setting key: base_url => baseUrl
  • Remove executable bit from files

see #31242.

#19 @pento
10 years ago

In 31745:

Add new smiley images missed in [31733].

See #31242

#20 @nacin
10 years ago

In 31746:

The partial change to the script handle in [31744] was not actually intended.

see #31242.

@pento
10 years ago

@iseulde
10 years ago

#21 @iseulde
10 years ago

MutationObserver! :)

@iseulde
10 years ago

#22 @iseulde
10 years ago

This will take care of infinite scroll, theme modal, media modal etc.

@iseulde
10 years ago

#23 @iseulde
10 years ago

We also don't need to parse any more in other files.

@iseulde
10 years ago

@iseulde
10 years ago

@iseulde
10 years ago

#24 @pento
10 years ago

In 31750:

Emoji: Instead of having custom hooks for Ajax callbacks, use MutationObserver to re-parse any changed elements in the DOM.

Props iseulde

See #31242

#25 @pento
10 years ago

In 31752:

Emoji: There's a little tear in my eye as I remove DOMDocument from the Emoji staticizer. It was a beautiful dream, but it wasn't to be.

Instead, let's use the tried and trusted smiley replacement algorithm, which has stood the test of time.

See #31242

@iseulde
10 years ago

@iseulde
10 years ago

#26 follow-up: @adamsilverstein
10 years ago

I was experiencing an Unresponsive script warning for twemoji until the last patches; testing with 31242.7.patch fixed the issue for me.

http://f.cl.ly/items/001g0B2A3w2K0I0D3l1Q/Monosnap_2015-03-12_09-41-46.jpg

#27 @iseulde
10 years ago

Returning false in the callback for the image src is a bad idea. We'd need to adjust this in twemoji.

#28 in reply to: ↑ 26 @johnbillion
10 years ago

Replying to adamsilverstein:

I was experiencing an Unresponsive script warning for twemoji until the last patches

For the record, I was seeing the same thing too.

#29 @iseulde
10 years ago

This is what's causing the freezing. Twemoji fails to replace with an image, text node is replaced anyway (it found a match), observer detects change, twemoji parses again... infinite loop.

@iseulde
10 years ago

#30 @iseulde
10 years ago

Temporary fix for now.

@iseulde
10 years ago

This ticket was mentioned in Slack in #core by azaozz. View the logs.


10 years ago

@iseulde
10 years ago

#33 @iseulde
10 years ago

Discussed this with @azaozz and we'll patch twemoji in core. All twemoji tests pass. See PR.

@azaozz
10 years ago

#34 @azaozz
10 years ago

In 31242.12.patch: also some old IE fixes.

#35 @azaozz
10 years ago

In 31756:

Emoji:

  • Patch twemoji.js to prevent infinite loop.
  • Fix some undefined errors in old IE.

Props iseulde. See #31242.

@iseulde
10 years ago

@iseulde
10 years ago

#36 @ocean90
10 years ago

In 31758:

Update Twemoji to 1.3.0.

Includes a fix for Firefox: https://github.com/twitter/twemoji/commit/92d5bea1abf0183fd00d306511c541b55b414959
Since this fix breaks IE 11 we added modified = false; after subnode.parentNode.replaceChild(fragment, subnode).

props iseulde.
see #31242.

#37 @ocean90
10 years ago

In 31759:

grunt imageminfor [31745].

see #31242.

#38 @azaozz
10 years ago

In 31761:

TinyMCE: fix emoji parsing in IE. See #31242.

#39 @azaozz
10 years ago

In 31762:

TinyMCE: fix emoji parsing in IE typo. See #31242.

@iseulde
10 years ago

#40 @iseulde
10 years ago

Update twemoji to 1.3.1, but still exclude select. Actually, about that: http://www.w3.org/TR/html-markup/option.html.
option can only contain a text node.
This is actually a weird territory. Emoji work in Chrome in select tags because it is native UI, even though Chrome does not support emoji. :)

#41 @iseulde
10 years ago

Another TinyMCE bug: linking some text with an emoji is not possible. The emoji just disappears.

#42 @boonebgorges
10 years ago

[31733] broke Tests_Post::test_utf8mb3_post_saves_with_emoji() for some setups. Here's my local environment:

$ mysql --version
mysql  Ver 15.1 Distrib 10.0.17-MariaDB, for Linux (x86_64) using readline 5.1

At least in this specific case - where mariadb versioning differs from mysql - the problem traces in part to https://core.trac.wordpress.org/browser/trunk/src/wp-includes/wp-db.php?marks=2804#L2780. $wpdb->has_cap( 'utf8mb4' ) is failing (even though my DB does in fact support utf8mb4), and this causes the test not to be skipped as it should (for reasons I have not dug into, but pento probably knows off the top of his head). I'm not sure why [31733] in particular broke this - I think it's the 'utf8' charset check in wp_insert_post()?

In any case, it's happening on Travis as well: https://travis-ci.org/aaronjorbin/develop.wordpress/jobs/54178126

#43 @azaozz
10 years ago

In 31766:

Emoji: update Twemoji to 1.3.1. Props iseulde. See #31242.

This ticket was mentioned in Slack in #meta by sergeybiryukov. View the logs.


10 years ago

#45 @SergeyBiryukov
10 years ago

31242.16.patch adds display: inline to Emoji styles, to avoid them being displayed as a block element (see the screenshot from the recent WordPress Core Weekly post).

#46 @SergeyBiryukov
10 years ago

In 31771:

Emoji and smiley images should be displayed inline.

see #31242.

@iseulde
10 years ago

#47 @iseulde
10 years ago

test is much faster than match.

@iseulde
10 years ago

#48 @azaozz
10 years ago

In 31772:

Emoji: better regex when testing for only char emoji. Props iseulde. See #31242.

#49 @azaozz
10 years ago

In 31773:

Emoji: yet another update for Twemoji, to 1.3.2. Props iseulde. See #31242.

@azaozz
10 years ago

#50 @azaozz
10 years ago

In 31242.19.patch:

  • Move the TinyMCE plugin css to wp-content.css.
  • Clean up both the plugin and wp-emoji.js, abstract and restructure a bit.

#51 @azaozz
10 years ago

In 31779:

Emoji:

  • Move the TinyMCE plugin CSS to wp-content.css.
  • Change the replacement images class to wp-emoji inside the editor.
  • Clean up both the plugin and wp-emoji.js, abstract and restructure a bit.

See #31242.

#52 @azaozz
10 years ago

In 31780:

Emoji: always export the methods in wp-emoji.js, even when dependencies are missing and we cannot initialize.
See #31242.

#53 @azaozz
10 years ago

We probably should change the class for the replacement images to wp-emoji. It is more compatible and future-proof. Any reasons not to?

Version 0, edited 10 years ago by azaozz (next)

#54 @pento
10 years ago

No reason, that was on my to-do list for next week - feel free to take care of it while you're powering through emoji, though. :-)

#55 @pento
10 years ago

In 31781:

Smilies: The new smilies added in [31733] and [31745] are larger than the old smilies. While this is taken care of by the CSS on normal pages, it means they're disproportionally large when seen in RSS and email.

By adding a little bit of inline style to them, we get pleasingly sized smilies everywhere. :-)

See #31242

#56 @azaozz
10 years ago

Having some second thoughts about changing the replacement images class name. Not sure how many places use Twemoji but it seems the .emoji class name may become the "de-facto standard" for these images.

One user case for keeping that unchanged is when pasting in the editor. If there are image tags with .emoji, we can easily find them and convert them (add the other attributes). Of course we can check for .wp-emoji too, but using .emoji will probably help make it into something like a de-facto standard.

#57 @azaozz
10 years ago

In 31786:

Emoji:

  • Add the styling for the replacement images to the admin CSS.
  • Revert to using .emoji as replacement image class.
  • When pasting in the editor, convert emoji images to our format so we can replace them with chars on saving.
  • Some more clean up of both the plugin and wp-emoji.js.

See #31242.

#58 @azaozz
10 years ago

In 31787:

Emoji: fix few typos. See #31242.

#59 @pento
10 years ago

In 31788:

Emoji: Move a comment to match the restructure in [31779], and add an extra comment explaining how emoji rendering support is detected.

See #31242

#60 @DrewAPicture
10 years ago

In 31789:

Clean up some inline documentation for emoji functionality, including a missing @since for mail_emoji(), and a changelog entry for wp_insert_post().

See #31242.

#61 follow-up: @DrewAPicture
10 years ago

  • Keywords commit removed

@pento: Can we come up with more descriptive names for feed_emoji() and mail_emoji()? I don't feel like they really fit with core style in that it's not immediately obvious what purpose they serve. Perhspa something like convert_feed_emoji() and convert_mail_emoji()?

#62 @SergeyBiryukov
10 years ago

In 31790:

Fix typo in a comment in [31752].

see #31242.

#63 in reply to: ↑ 61 ; follow-up: @pento
10 years ago

Replying to DrewAPicture:

@pento: Can we come up with more descriptive names for feed_emoji() and mail_emoji()? I don't feel like they really fit with core style in that it's not immediately obvious what purpose they serve. Perhspa something like convert_feed_emoji() and convert_mail_emoji()?

For you, @DrewAPicture, we can absolutely do that.

If we want to chat about purpose before deciding on a name, it's worth considering that they're really just single-purpose filters (to be thin wrappers that filter content through wp_staticize_emoji()), so maybe they're better as _-prefixed @ignore-d functions?

Perhaps _wp_staticize_emoji_for_email() and _wp_staticize_emoji_for_feeds()?

#64 in reply to: ↑ 63 @DrewAPicture
10 years ago

Replying to pento:

If we want to chat about purpose before deciding on a name, it's worth considering that they're really just single-purpose filters (to be thin wrappers that filter content through wp_staticize_emoji()), so maybe they're better as _-prefixed @ignore-d functions?

Yes. Yes.

Perhaps _wp_staticize_emoji_for_email() and _wp_staticize_emoji_for_feeds()?

Yes.

#65 @pento
10 years ago

In 31791:

Emoji: Rename the email and feed filter functions to be _ prefixed, and @ignore-d in the PHPDocs.

See #31242

This ticket was mentioned in Slack in #core by pento. View the logs.


10 years ago

#67 @pento
10 years ago

  • Keywords has-patch removed
  • Resolution set to fixed
  • Status changed from assigned to closed

*ksssshhhhtttt* This is your Emoji Captain speaking, it's been a pleasure flying you to Emoji Island, all of us here at x1f4a9 Airlines hope your stay is a pleasant one.

If you have any feedback for us, please open a new ticket at your leisure.

#68 @nacin
10 years ago

In 32161:

Clean up wp_staticize_emoji() and friends.

  • DOMDocument was removed in [31752] but not the check.
  • wp_staticize_emoji() has never accepted a second arg; remove it from calls.
  • Remove wp_staticize_emoji_for_feeds(), no need for it.
  • Remove _ and @ignore from wp_staticize_emoji_for_email(), no need for it.

see #31242.

Note: See TracTickets for help on using tickets.