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

ui: Remove "Noto Color Emoji", broken on iOS (and maybe Android?) #403

Merged
merged 1 commit into from
Nov 21, 2023

Conversation

chrisbobbe
Copy link
Collaborator

@chrisbobbe chrisbobbe commented Nov 21, 2023

Details in the commit message.

tl;dr:

When this Flutter issue is resolved:
  flutter/flutter#134897
try adding the Noto Color Emoji font back again (COLRv1 format) and
set it as a fallback font behind some higher-priority font wherever
we want to show text that might contain emojis.

Before we do that, we're at the mercy of the system font, which
might not be updated to handle newer emojis. This seems especially
true of Android devices in the wild.

-----

In zulip#245, we added a COLRv1 version of this font, probably because of
a warning that made us doubt if other formats would work on iOS:
  https://github.com/googlefonts/noto-emoji#using-notocoloremoji

> NotoColorEmoji uses the CBDT/CBLC color font format, which is
> supported by Android and Chrome/Chromium OS. Windows supports it
> starting with Windows 10 Anniversary Update in Chrome and Edge. On
> macOS, only Chrome supports it, while on Linux it will support it
> with some fontconfig tweaking, see issue zulip#36. Currently we do not
> build other color font formats.

(It seems like that part of the README wasn't updated when the
COLRv1 format was added, and hasn't been updated since.)

And I guess there wasn't a clear iOS compatibility problem with the
COLRv1 format, so we used that.

But, in zulip#245, it seems like we didn't do a good manual test that the
COLRv1 version of this font could actually handle emojis in the app
on iOS. That testing shows that it can't (or at least couldn't on my
phone or in a simulator). When the font is in the effective list of
fallbacks, it takes responsibility for rendering the emoji, before
the system font gets a chance to, but then it fails to render it,
and a blank space appears. We suspect this Flutter issue ("Rendering
of COLRv1 fonts is broken"):
  flutter/flutter#134897

We don't know why that Flutter issue doesn't, or at least doesn't
always, prevent emojis from successfully rendering in the font on
*Android*. I followed the issue's reproduction recipe with the
COLRv1 font they specified, and indeed the glyphs weren't showing up
on Android (but they did on web). So apparently the specific COLRv1
font is a variable.

Since we don't know what all the variables are, and we have other
short-term priorities, just take the font out of the picture for
now, with a plan to reinstate it once we're sure it won't cause
emojis to fail to render.

Discussion:
  https://chat.zulip.org/#narrow/stream/243-mobile-team/topic/Emoji.20rendering.20issue.20.28iOS.29/near/1683965
@chrisbobbe chrisbobbe added a-content Parsing and rendering Zulip HTML content, notably message contents a-design Visual and UX design labels Nov 21, 2023
@gnprice
Copy link
Member

gnprice commented Nov 21, 2023

Thanks for figuring this out! Looks good; merging.

I also left a comment flutter/flutter#134897 (comment) on that upstream issue, with a digest of what you'd found.

I'll file a followup issue for us to track shipping an emoji font again ourselves. → #404

@gnprice gnprice merged commit a1da95a into zulip:main Nov 21, 2023
1 check passed
@chrisbobbe chrisbobbe deleted the pr-remove-noto-color-emoji branch November 27, 2023 20:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
a-content Parsing and rendering Zulip HTML content, notably message contents a-design Visual and UX design
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants