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

bbPress: emojis causing stray img tags to display #135

Open
kathrynwp opened this issue Jan 16, 2023 · 34 comments
Open

bbPress: emojis causing stray img tags to display #135

kathrynwp opened this issue Jan 16, 2023 · 34 comments
Labels
bbpress bug Something isn't working

Comments

@kathrynwp
Copy link
Member

Emojis are sometimes causing closing </img> tags to appear in the WP.org forums.

Live example:

https://wordpress.org/support/topic/front-page-74/#post-16371055

Front_page___WordPress_org

I also did some testing on the test site, adding emojis in three different ways. Here's the thread:

https://test.wordpress.org/support/topic/emoji-test/

Related: #111

@kathrynwp kathrynwp added bug Something isn't working bbpress labels Jan 16, 2023
@johngodley
Copy link
Member

How are you getting this to happen? #121 was added a month ago that strips out the </img> tags and this is working fine in all the tests I perform. I know that wp.org has some additional code (#111) but that doesn't seem to cause this issue either.

@kathrynwp
Copy link
Member Author

Hey there @johngodley !

This test page shows the stray closing img tags and the circumstances under which they appear:

https://test.wordpress.org/support/topic/emoji-test/

It happens when:

  1. using the emoji picker in Firefox (though it happened once out of the two tests you see there)

  2. copy-pasting from an external page - https://emojiguide.com/smileys-emotion/face-with-open-mouth/

I did not get the stray tags when typing an emoticon like :-) and letting the forum convert it to an emoji.

Please let me know if I can help in any other way. Thank you!

@johngodley
Copy link
Member

Yep I saw the test page but I've not been able to reproduce it myself.

By the emoji picker in Firefox do you mean the system 'emoji and symbols' picker? I can't find anything specific to Firefox but it's not a browser I use so I might be missing it.

image

@kathrynwp
Copy link
Member Author

By the emoji picker in Firefox do you mean the system 'emoji and symbols' picker?

Yes, exactly!

Edit_and_Menubar_and_bbPress__emojis_causing_stray_img_tags_to_display_·_Issue__135_·_Automattic_blocks-everywhere

Firefox 108.0.2
macOS 12.6

I made a screen recording (with audio) in case it helps you replicate:

forum-emoji.mov

@Otto42
Copy link

Otto42 commented Jan 19, 2023

Also happens on windows by using the win+period key to insert emoji.

@kathrynwp
Copy link
Member Author

Flagging this other emoji issue:

https://meta.trac.wordpress.org/ticket/6703

@johngodley
Copy link
Member

I've released 1.14.2 which I think should fix this.

I don't understand what https://meta.trac.wordpress.org/ticket/6703 means @kathrynwp, and the referenced report doesn't exist (for me anyway). Do you have any more details? If it's not the same issue here then it may be worth creating a new one.

@kathrynwp
Copy link
Member Author

Thank you so much, @johngodley !

Good news: the issue looks to be resolved with emojis that are added from the system menu.

I'm still seeing the same problem with copied emojis, but not sure how much of an issue that is. See my two test threads starting here: https://test.wordpress.org/support/topic/emoji-test/?view=all#post-11657373

I don't understand what https://meta.trac.wordpress.org/ticket/6703 means @kathrynwp, and the referenced report doesn't exist (for me anyway). Do you have any more details? If it's not the same issue here then it may be worth creating a new one.

I'm going to flag this one for Marius to look at, as I don't have more details. Thanks!

@johngodley
Copy link
Member

I'm still seeing the same problem with copied emojis, but not sure how much of an issue that is. See my two test threads starting here: https://test.wordpress.org/support/topic/emoji-test/?view=all#post-11657373

I don't think it's been updated yet on wp.org.

@Clorith
Copy link

Clorith commented Jan 24, 2023

An interesting ticket, that may somehow be related to this, did land in core a few days back, cross-referencing here for the sake of completeness https://core.trac.wordpress.org/ticket/57517

What I suspect may be happening (and this may have other inverse effects in our forum scenario actually, where we'd need this to not happen), is that the twemoji has a global observer, and any time the DOM is updated, will replace emoji with a corresponding img tag. Given how the block editor introduces new DOM nodes, and they go in and out of editable mode, this may be causing some of the issues being observed here (including another ticket we have where emojis in reviews are now breaking the ability to post reviews, as reviews are hardcoded ot not allow links in them 😅).

This is just a theory, as I've not had a chance to fully test it out locally yet (time constraints), but I wanted to sahre the discovery at least since you've been trying to track things down. If we could somehow have the twemoji fire once on DOM ready, and then have the global observer be killed, so it won't affect what's going on inside the editor afterwards, that would be ideal, as it would provide the original benefits on page load, but not interfere with anything else (I suspect this may come up if someone uses the block editor elsewhere as well via the blocks everywhere approach).

@porg
Copy link

porg commented Jan 31, 2023

On https://test.wordpress.org/support/topic/emoji-test this is still not fixed.

In my recent test series from today some instances work and some fail.

I do not recognize yet what makes the difference between success and failure.

@johngodley
Copy link
Member

I'm not able to reproduce the problem with this plugin on a vanilla WP so I can only assume there is some difference on wp.org, which I don't have access to.

Rather than continuing down the 'trying to fix the content after it's broken' route I've added a new patchEmoji option in 1.14.3 which monkey-patches twemoji and prevents it affecting the editor. We can see how that goes before looking at maybe putting something similar in core WP.

@porg
Copy link

porg commented Feb 1, 2023

Sorry my test postings, which show different outcomes for the same Emoji, are still in the moderation queue.

When they get finally published, you can see that, and maybe realize what the crucial difference may be.

@johngodley
Copy link
Member

When they get finally published, you can see that, and maybe realize what the crucial difference may be.

It won't help any more than the other posts, but thanks.

@porg
Copy link

porg commented Feb 1, 2023

The test posts are now online. Despite unlikely, maybe it still brings some insight.

@Clorith
Copy link

Clorith commented Feb 2, 2023

Noting that the ability to skip the wpemoji handling landed in core yesterday, we if we can pass a wrapper element around the block editor with the correct class, we should (in theory at least) be able to skip it's processing of DOM nodes and hopefully address this in a more direct way?

https://core.trac.wordpress.org/changeset/55186

@johngodley
Copy link
Member

Thanks @Clorith that looks ideal! I'll add the class names in the next version.

@porg
Copy link

porg commented Feb 21, 2023

I discovered a few new aspects of the bug (with the forum as of today):

Unicode Emoji Symbols in Support forums with Gutenberg Block Editor

Will your fix cover all those aspects?

@porg
Copy link

porg commented Feb 21, 2023

Note: If you want to inspect the post from the screenshot in markup or database representation it is / will be here. My posts on WP.org appear only delayed. I have a moderation flag since over a year already and the WP forum moderators say it will be lifted one day, but they still need more sample data.

@johngodley
Copy link
Member

The wp-exclude-emoji class has been added so possibly this is now fixed (assuming wp.org is running the latest WP code, which I think it is)

@porg
Copy link

porg commented Mar 3, 2023

Latest test still shows the same buggy behavior.

@porg
Copy link

porg commented Mar 22, 2023

Still happened on WP.org support forum today.

@porg
Copy link

porg commented Mar 25, 2023

I realized one more thing which may make a difference:

  • In some blocks where ENTER creates a new sibling block within a parent block, e.g. in a <ul> unsorted list ENTER creates a new <li> element.
    • ✅ There the Unicode to inline SVG icon replacement happens as soon as you have pressed ENTER.
  • In other blocks where ENTER creates a paragraph break (one p to the next p element):
    • ❗️ ENTER alone does not trigger a replacement.
    • 💡 But for those SHIFT-ENTER (which inserts a line break <br> ) the Unicode glyph gets replaced to the inline SVG icon.

Quite many operations run when hitting ENTER in the Block Editor. Maybe the cause for this lies that not all blocks are treated the same by these methods, which leads to the disparities regarding Unicode replacement.

See my video recording (4min, no audio) — Easily fast forward as needed and you still get the gist of it:

Emoji.replacement.differs.by.block.type.and.between.ENTER.and.SHIFT-ENTER.mp4

@porg
Copy link

porg commented Mar 25, 2023

In the published version the stray tags only happened on all instances where SHIFT-ENTER was used. There the replacement worked fine in the Editor environment. But after publishing they left the stray tag.

@porg
Copy link

porg commented Apr 4, 2023

Can someone please verify/falsify that the ENTER event makes the crucial difference?

@porg
Copy link

porg commented Apr 28, 2023

Side note:

  • The guidelines for WP.org plugin reviews forbid the use of links.
  • When you try to submit a review with an emoji, behind the scenes the emoji gets an icon block which has a link in it.
  • Hence the submission form warns you to not submit a review with a link (although the user included no link consciously).
  • On the 3rd submission attempt the message gets into the moderation queue. The user has no feeling of wrongdoing. (the link generation happens in the dark behind the user's eyes)

@dd32
Copy link
Member

dd32 commented May 2, 2023

I've made some changes via https://meta.trac.wordpress.org/ticket/6964 - I think this will fix the majority of the support forum issues, and I think this plugin has done everything within it's control (Adding the class, and then letting the bbPress install do what it wants). It looks like some Browsers might be choking on the emoji's still, and ignoring the class, so that might be a bug that needs to be reported to Core (But it might also be expected, as it depends on how Gutenberg works).

Half the discussion in this issue seems like stuff that wasn't related to this plugin at all, some aspects of how Gutenberg does what it does is unescapable by this plugin. Some of the aspects of how Twemoji works is also out of our control.

@porg
Copy link

porg commented Jul 10, 2023

Situation worsened big time!

@porg
Copy link

porg commented Jul 10, 2023

Dear support forum maintainer:
Prior rollout of forum software upgrades please check the rendering of some sample/test content:

  • Text formatting
  • Inline emojis
  • Supported blocks (Image, Imgurl, Youtube, Vimeo, etc)

@porg
Copy link

porg commented Jul 18, 2023

Full width SVG emojis got fixed 1-2 days after my report.

@dd32
Copy link
Member

dd32 commented Jul 19, 2023

I've added some base-styles for emoji fallback images in WordPress/wordpress.org@d9985c7 which resolved some of these issues.

I suspect some of these issues were unrelated to Blocks Everywhere, and some were, confusion lead to everything being blamed on one thing where it actually had multiple aspects to it.

@dd32
Copy link
Member

dd32 commented Apr 18, 2024

Just documenting that this was resolved on the WordPress.org support forums, but probably still exists in the plugins bbPress handler

The wp-exclude-emoji class still isn't enough to cause the core emoji handler to kick in.
https://meta.trac.wordpress.org/changeset/12568 + https://meta.trac.wordpress.org/changeset/12569 worked around it for the forums, by replacing it upon save.

I've been using Firefox ESR 115 on a Linux virtual machine for testing emojis when I need to trigger the code.

@porg
Copy link

porg commented Apr 19, 2024

✅ Test was successful

  • Created https://test.wordpress.org/support/topic/emoji-test-2024-04/
  • On WP.org the Unicode Emojis get transformed to WordPress icon markup:
    • 1: Already while in the Editor during certain text editing operations (which JavaScript monitors):
      • Not when the unicode symbol is entered on the fly.
      • Not when a line with an Emoji moves one paragraph down.
      • But when a line with an Emoji moves one paragraph up then the emoji gets transformed.
    • 2: At latest after submission.
      • When reading the published post all Unicode glyphs got transformed to WP icon markup.

@porg
Copy link

porg commented Apr 19, 2024

WordPress.Forums.bbPress.Emoji.Test.2024-04-19.mp4

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bbpress bug Something isn't working
Projects
None yet
Development

No branches or pull requests

6 participants