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

Block ™️, ©️ from emojification #1234

Closed
wants to merge 3 commits into from

Conversation

riking
Copy link

@riking riking commented Apr 8, 2017

Forbid ™ ® © so they don't turn into black-on-black emoji.

I haven't figured out the docker-compose stuff yet so these changes haven't been tested yet. If someone else could make sure this runs that would be nice.

@riking riking changed the title Blacklist ™️, ©️ from emojification Block ™️, ©️ from emojification Apr 8, 2017
@krainboltgreene krainboltgreene added the ui Front-end, design label Apr 8, 2017
@ineffyble
Copy link
Member

Is there a better option? Such as using a light/white version of the EmojiOne images?

@riking
Copy link
Author

riking commented Apr 8, 2017

Emojione doesn't seem to provide that from what I can tell.

This is also an issue for emoji such as ✔️ ➗ 💲 ➖ ✖️ but those don't have quite as widespread font support for the Unicode versions.

Copy link
Contributor

@yiskah yiskah left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This does not seem like a good solution for this problem. What will happen is instead of returning an intended look it will return the string ":tm:" etc. which will be perceived as an error.

@riking
Copy link
Author

riking commented Apr 10, 2017

Really? In my browser, the content of the alt attribute is , not :tm:.

@marrus-sh
Copy link
Contributor

marrus-sh commented Apr 17, 2017

The Unicode solution for this is to use the Emoji Variation Selector U+FE0E to select text representations for glyphs; Apple etc will insert these for you whenever you insert a text version of an emoji, however EmojiOne doesn't respect them (joypixels/emojione#301, joypixels/emojione#415). Furthermore, EmojiOne defaults to emoji representations for all characters, including a number of characters which should (by the Emoji spec recommendations for web pages) default to text (for example, ™, ®, ©; again see joypixels/emojione#415). So this PR is actually a workaround for EmojiOne's bug and a good approach IMO; the right-hand column of this chart provides other characters which predate emoji or otherwise might be likely to have text representations. Of course, it would be up to us as to which (if any) of these we would implement.

(See also #349, which is the Mastodon version of this Issue before we realized it was on EmojiOne's side.)

I agree that using the alt attribute is less-than-ideal though; I don't have push access to this PR but the fix that I would suggest is here: https://gist.github.com/marrus-sh/71b3b4813e648ae4263e6a4e4b78c19a; note that this doesn't affect emoji inserted with shortnames at all (the assumption being that if you're using a shortname then you intended the emoji presentation), however I think Masto converts shortnames to character points now anyway so maybe that doesn't even matter. All I did was add an extra check to the str.replace function.

@marrus-sh
Copy link
Contributor

This is something of a separate issue, but with respect to dark emoji not being visible, see also #317, especially this comment where I suggest using EmojiOne's Project B&W emoji for otherwise-unreadable emoji forms. Mastodon already uses SVGs for its emoji, so the changes that would need to take place would be as follows:

  1. Tell Mastodon to pull from Project B&W SVGs instead of regular ones for dark emoji
  2. Insert emoji using an <svg> element directly (through <use fill="currentColor" xlink:href="[emoji path]"> or similar) instead of through <img> and style that element appropriately. I'm like 75% sure that this will work. If not, then the emoji would appear as plain black, which would be the opposite of help.

@ticky
Copy link
Contributor

ticky commented Apr 23, 2017

It looks like emojione 3.0 fixes this (joypixels/emojione#415), but we can’t use it until emojione-picker updates its dependency.

@mjankowski
Copy link
Contributor

I agree that we should let the supporting lib fix this problem and benefit from it when they do.

@mjankowski mjankowski closed this Apr 24, 2017
abcang added a commit to pixiv/mastodon that referenced this pull request Feb 25, 2019
Limit statuses subquery in Account.triadic_closures
dariusk added a commit to friendcamp/mastodon that referenced this pull request Dec 31, 2022
Fixes mastodon#1234 by adding language to the preferences page. It's only in English, unfortunately.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ui Front-end, design
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants