Make WordPress Core

Opened 5 years ago

Closed 5 years ago

Last modified 5 years ago

#48293 closed feature request (fixed)

Update Twemoji to version 12.1.3

Reported by: desrosj's profile desrosj Owned by: desrosj's profile desrosj
Milestone: 5.3 Priority: normal
Severity: normal Version:
Component: Emoji Keywords: has-patch good-first-bug commit dev-reviewed
Focuses: Cc:

Description

This version includes adjustments to a handful of emoji.

Release notes: https://github.com/twitter/twemoji/releases/tag/v12.1.3

Previously: #47852, #46805, #45133.

Attachments (1)

48293.patch (1.6 KB) - added by matstars 5 years ago.
diff patch for 48293

Download all attachments as: .zip

Change History (19)

#1 @desrosj
5 years ago

  • Type changed from defect (bug) to feature request

#2 @desrosj
5 years ago

  • Keywords good-first-bug added

Adding good-first-bug.

Also, just wanted to note that the assets would need to be updated on .org when this is committed.

#3 @matstars
5 years ago

I can work on this during contributor day

#4 @joemcgill
5 years ago

  • Owner set to matstars
  • Status changed from new to assigned

Thanks @matstars!

#5 @matstars
5 years ago

@desrosj @pento @dd32

The latest version of Twemoji (https://github.com/twitter/twemoji/releases/tag/v12.0.4) includes some replacement emojis and no new ones. This is validated by doing a diff versus the latest s.w.org CDN's https://s.w.org/images/core/emoji/12.0.0-1/72x72 and https://s.w.org/images/core/emoji/12.0.0-1/svg (analogous) directories compared to the Twemoji release, and seeing that it's all modifications (no adds or deletes).

Looks like to me that we have two paths that we can go down:

  1. We could replace the assets from the latest Twemoji release in the path that is serving the current CDN, i.e. https://s.w.org/images/core/emoji/12.0.0-1/72x72 and https://s.w.org/images/core/emoji/12.0.0-1/svg and then, if possible, purge the CDN cache on all URLs beginning with those paths. Even if we do this, given the current cache TTL is ten years (cache-control: max-age=315360000). Anything that has the assets cached will not see the changed emoji for a long time without any other intervention. This is rather easy I'd imagine, would require updating the 22 or so diffed assets in place where the current assets are stored that are being served by CDN. After doing that, I believe the only update in code would be changing the package.lock and re-running npm install.
  1. We can add the whole updated image-set including the 12.1.3 changes into a new directory on the CDN, i.e. https://s.w.org/images/core/emoji/12.1.3/72x72/ and https://s.w.org/images/core/emoji/12.1.3/svg/ and serve the new assets from there -- a corresponding update of the URLs being referenced need to be updated, i.e. replace the four or so places where the 12.0.0-1 is referenced in code, e.g.,
'baseUrl' => apply_filters( 'emoji_url', 'https://s.w.org/images/core/emoji/12.0.0-1/72x72/' ),

to

'baseUrl' => apply_filters( 'emoji_url', 'https://s.w.org/images/core/emoji/12.1.3/72x72/' ),

along with updating the package.lock and re-running npm install. Without any institutional knowledge, the second option makes the most sense as it is cut and dry and less likely to see inconsistencies, but I am not sure if doing so would be labor intensive or would require any additional work.

Let me know your thoughts.

Last edited 5 years ago by matstars (previous) (diff)

#6 @pento
5 years ago

  • Milestone changed from 5.4 to 5.3

The 12.0.0-1 CDN folder currently includes everything up to Twemoji version 12.1.2 (which, in an unfortunate versioning coincidence, doesn't correspond to the Emoji 12.1 spec). So, the only changes to be concerned about are those in the Twemoji 12.1.3 release:

  • Trans flag: This was added in Twemoji 12.1, so currently only works in WordPress 5.3. I'm fine with the older version appearing for the handful of people who may have it cached already.
  • Rosetta, ballot box, puzzle piece: All from older Emoji specs, so are probably cached by a lot of people. The appearance has been polished, but the meaning hasn't been changed. I don't think there's a risk of folks seeing the older/newer versions depending on what they have cached.
  • Woman/Man/Person playing handball: From an older emoji spec. The figure has changed direction, but is otherwise the same. Given that other vendor implementations vary in what direction they face, I don't think this is a major concern. (Note: the Twemoji 12.1.3 changelog incorrectly shows that the man playing handball has changed to using the figure of the woman playing handball.)

Provided this can be done in WP 5.3, I'm inclined to think that just overwriting the 12.0.0-1 folder on the CDN, and upgrading the Twemoji package version is fine.

I'll move this ticket to the 5.3 milestone for consideration, but it's up to the 5.3 release leads as to whether they want to merge it this late in the cycle.

@matstars
5 years ago

diff patch for 48293

#7 @matstars
5 years ago

in that case, I believe this patch is all that's needed @pento -- we also need to coordinate updating the CDN with the SVG/PNG files from https://github.com/twitter/twemoji SVG and 72x72

Last edited 5 years ago by matstars (previous) (diff)

#8 @Otto42
5 years ago

Change as described by @pento above committed and deployed in [dotorg:15520]. cc @desrosj

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


5 years ago

#10 @johnbillion
5 years ago

  • Owner changed from matstars to desrosj

#11 @desrosj
5 years ago

  • Resolution set to fixed
  • Status changed from assigned to closed

In 46665:

Emoji: Upgrade Twemoji to 12.1.3.

This point release does not add any images and only slightly modifies a few images without changing their meanings. Because of this, the same CDN location can be used and has been updated.

Props matstars, desrosj, Otto42, pento.
Fixes #48293.

#12 @desrosj
5 years ago

  • Keywords commit dev-feedback added
  • Resolution fixed deleted
  • Status changed from closed to reopened

Reopening to backport.

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


5 years ago

#14 @SergeyBiryukov
5 years ago

  • Keywords has-patch dev-reviewed added; needs-patch dev-feedback removed

Looks good to backport.

#15 @desrosj
5 years ago

  • Resolution set to fixed
  • Status changed from reopened to closed

In 46667:

Emoji: Upgrade Twemoji to 12.1.3.

This point release does not add any images and only slightly modifies a few images without changing their meanings. Because of this, the same CDN location can be used and has been updated.

Merges [46665] to the 5.3 branch.

Reviewed by desrosj, SergeyBiryukov.
Props matstars, desrosj, Otto42, pento.
Fixes #48293.

#16 @desrosj
5 years ago

In 46668:

Bundled Themes: Update Twenty Twenty.

This brings Twenty Twenty in sync with GitHub. For a full list of changes since 5.3 RC3, see https://github.com/WordPress/twentytwenty/compare/dea9290...c267289.

Props williampatton, poena, andersnoren.
See #48110, #48386, #48450, #48293.
Fixes #48505.

#17 @desrosj
5 years ago

In 46669:

Bundled Themes: Update Twenty Twenty.

This brings Twenty Twenty in sync with GitHub. For a full list of changes since 5.3 RC3, see https://github.com/WordPress/twentytwenty/compare/dea9290...c267289.

Props williampatton, poena, andersnoren.
See #48110, #48386, #48450, #48293.
Fixes #48505.

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


5 years ago

Note: See TracTickets for help on using tickets.