Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Emoji script needs to be disabled/improved #2799

Closed
ellatrix opened this issue Sep 26, 2017 · 5 comments
Closed

Emoji script needs to be disabled/improved #2799

ellatrix opened this issue Sep 26, 2017 · 5 comments
Assignees
Labels
[Type] Bug An existing feature does not function as intended [Type] WP Core Ticket Requires an upstream change from WordPress. Core Trac ticket should be linked.

Comments

@ellatrix
Copy link
Member

ellatrix commented Sep 26, 2017

Issue Overview

The emoji script, which is enqueued on all front-end and admin pages, and replaces emoji with images in browsers that have poor support (Chrome still too apparently), both at page load but also on mutations, is not compatible with Gutenberg's Editable at the moment.

Steps to Reproduce (for bugs)

  1. Use Chrome.
  2. Insert an emoji (script does not catch it).
  3. Move the block down (now script does catch it and you'll see an image instead).
  4. Go to text mode and observe how there's an image instead of an emoji.

Expected Behavior

  1. Emoji are saved as emoji, not images.
  2. Script should not mess with undo levels etc.

Current Behavior

The opposite. :)

Possible Solution

  1. Remove the script on this page. Maybe we even have concerns with this generally in combination with React. Cc @aduth @youknowriad.
  2. Add the exclude class. This does not seem to work though, it only looks at the top mutated node. This seems to be a core bug.
  3. Add the TinyMCE emoji plugin as it is added to the WP core editor.
  4. Reconsider the script entirely. Is it still needed? Is Chrome (etc.) support really that bad? I was hoping after some time we might be able to drop it from WP. Cc @pento.

Related Issues and/or PRs

@ellatrix ellatrix added [Component] TinyMCE [Type] Bug An existing feature does not function as intended [Type] Question Questions about the design or development of the editor. [Type] WP Core Ticket Requires an upstream change from WordPress. Core Trac ticket should be linked. labels Sep 26, 2017
@ellatrix ellatrix added this to the Beta, Needs to happen milestone Sep 26, 2017
@pento
Copy link
Member

pento commented Sep 28, 2017

The emoji script will likely be around forever - Twemoji tends to update their assets very quickly after a new Emoji spec is released, which we can start using immediately for rendering purposes.

OS support follows sporadically, and then relies on people actually performing the update. To take the most recent Emoji update as an example, the spec was released May 18, the Twemoji update came up May 24, Core update May 25.

Android added support in Android 8.0, released August 21. Windows, iOS, and MacOS support will be sometime later this year.

@ellatrix
Copy link
Member Author

Okay, guess we should go for option 3 then, plus a fix on the exclude class in core. Still wondering if the script in general is fine in React, updating text nodes outside editable areas. Theoretically it could just see the nodes again as dirty, which would again trigger a change by the script, creating an infinite loop.

@pento
Copy link
Member

pento commented Nov 1, 2017

This also causes problems for RSS feeds - the code that replaces emoji with static images in feeds adds size styling, so we don't have ridiculously huge emoji appearing in feeds. It doesn't run on the image version of emoji. :-)

GIANT 🎃

@ellatrix ellatrix self-assigned this Nov 1, 2017
@mtias mtias modified the milestones: Merge Proposal, WordPress 5.0 Mar 8, 2018
@ellatrix
Copy link
Member Author

ellatrix commented Mar 8, 2018

Per chat, let's disable the script on the page to resolve both editable problems and React concerns.

@jeffpaul
Copy link
Member

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

@jasmussen jasmussen removed the [Type] Question Questions about the design or development of the editor. label Apr 2, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Type] Bug An existing feature does not function as intended [Type] WP Core Ticket Requires an upstream change from WordPress. Core Trac ticket should be linked.
Projects
None yet
Development

No branches or pull requests

5 participants