Font Library: install fonts in sequence to work around race condition #60180
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
creativecoder
added
[Type] Bug
creativecoder
added
the
Backport to WP Beta/RC
creativecoder
requested review from
getdave,
mcsf,
matiasbenedetto and
mikachan
mcsf
approved these changes
Mar 25, 2024
There was a problem hiding this comment.
LGTM, but let's get some additional testing, and I want @matiasbenedetto's opinion too.
|
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message. To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
|
Size Change: +62 B (0%) Total Size: 1.72 MB
ℹ️ View Unchanged
|
|
Great! This seems to have resolved the issue with the PR.
|
peterwilsoncc
approved these changes
Mar 25, 2024
There was a problem hiding this comment.
This looks good and tests well. The timeline in the browser tools' network tab confirms the uploads are sequential.
I was able to consistently reproduce the race condition with a version of the code in #59023 (comment)
packages/edit-site/src/components/global-styles/font-library-modal/utils/index.js
Outdated
Show resolved
Hide resolved
getdave
added
the
Backport to Gutenberg RC
|
@bph we'll need to bring this into Gutenberg 18.0.0. |
|
This PR was triaged by Editor release leads and contributors in WP Slack as part of the final WordPress 6.5 process. See https://wordpress.slack.com/archives/C02QB2JS7/p1711371934226359 (requires registration). The criteria used was:
Conclusion: we propose that this fix is included in WP 6.5. Reason: bug affects the (blessed) Font Library feature in a severe way and this PR resolves it in a simple fashion. We now await a final decision from the full release team. |
Co-authored-by: Peter Wilson <519727+peterwilsoncc@users.noreply.github.com>
getdave
approved these changes
Mar 26, 2024
|
There have now been multiple reviews and tested by contributors. As a result I went ahead and merged this. |
bph
pushed a commit
that referenced
this pull request
Mar 26, 2024
…#60180) * Font Library: install fonts in sequence to work around font directory race condition * Update comment block to coding standards Co-authored-by: Peter Wilson <519727+peterwilsoncc@users.noreply.github.com> * Fix linting --------- Co-authored-by: creativecoder <grantmkin@git.wordpress.org> Co-authored-by: getdave <get_dave@git.wordpress.org> Co-authored-by: mcsf <mcsf@git.wordpress.org> Co-authored-by: peterwilsoncc <peterwilsoncc@git.wordpress.org> Co-authored-by: mikachan <mikachan@git.wordpress.org> Co-authored-by: huzaifaalmesbah <huzaifaalmesbah@git.wordpress.org> Co-authored-by: t-hamano <wildworks@git.wordpress.org> Co-authored-by: swissspidy <swissspidy@git.wordpress.org> Co-authored-by: kjnanda <krupajnanda@git.wordpress.org> Co-authored-by: matiasbenedetto <mmaattiiaass@git.wordpress.org>
|
I just cherry-picked this PR to the release/18.0 branch to get it included in the next release: 671b80a |
bph
removed
the
Backport to Gutenberg RC
|
@creativecoder Please could we look into adding some test coverage for the changes in this PR. This will increase confidence of release team in including this in WP 6.5. See related Slack discussion. |
getdave
pushed a commit
that referenced
this pull request
Mar 27, 2024
…#60180) * Font Library: install fonts in sequence to work around font directory race condition * Update comment block to coding standards Co-authored-by: Peter Wilson <519727+peterwilsoncc@users.noreply.github.com> * Fix linting --------- Co-authored-by: creativecoder <grantmkin@git.wordpress.org> Co-authored-by: getdave <get_dave@git.wordpress.org> Co-authored-by: mcsf <mcsf@git.wordpress.org> Co-authored-by: peterwilsoncc <peterwilsoncc@git.wordpress.org> Co-authored-by: mikachan <mikachan@git.wordpress.org> Co-authored-by: huzaifaalmesbah <huzaifaalmesbah@git.wordpress.org> Co-authored-by: t-hamano <wildworks@git.wordpress.org> Co-authored-by: swissspidy <swissspidy@git.wordpress.org> Co-authored-by: kjnanda <krupajnanda@git.wordpress.org> Co-authored-by: matiasbenedetto <mmaattiiaass@git.wordpress.org>
|
e2e test coverage is being worked on in #60221. |
carstingaxion
pushed a commit
to carstingaxion/gutenberg
that referenced
this pull request
Mar 27, 2024
…WordPress#60180) * Font Library: install fonts in sequence to work around font directory race condition * Update comment block to coding standards Co-authored-by: Peter Wilson <519727+peterwilsoncc@users.noreply.github.com> * Fix linting --------- Co-authored-by: creativecoder <grantmkin@git.wordpress.org> Co-authored-by: getdave <get_dave@git.wordpress.org> Co-authored-by: mcsf <mcsf@git.wordpress.org> Co-authored-by: peterwilsoncc <peterwilsoncc@git.wordpress.org> Co-authored-by: mikachan <mikachan@git.wordpress.org> Co-authored-by: huzaifaalmesbah <huzaifaalmesbah@git.wordpress.org> Co-authored-by: t-hamano <wildworks@git.wordpress.org> Co-authored-by: swissspidy <swissspidy@git.wordpress.org> Co-authored-by: kjnanda <krupajnanda@git.wordpress.org> Co-authored-by: matiasbenedetto <mmaattiiaass@git.wordpress.org>
getdave
added
Backported to WP Core
|
I just cherry picked this into WP 6.5 RC 4 branch a5ab3c5 |
getdave
pushed a commit
that referenced
this pull request
Mar 27, 2024
…#60180) * Font Library: install fonts in sequence to work around font directory race condition * Update comment block to coding standards Co-authored-by: Peter Wilson <519727+peterwilsoncc@users.noreply.github.com> * Fix linting --------- Co-authored-by: creativecoder <grantmkin@git.wordpress.org> Co-authored-by: getdave <get_dave@git.wordpress.org> Co-authored-by: mcsf <mcsf@git.wordpress.org> Co-authored-by: peterwilsoncc <peterwilsoncc@git.wordpress.org> Co-authored-by: mikachan <mikachan@git.wordpress.org> Co-authored-by: huzaifaalmesbah <huzaifaalmesbah@git.wordpress.org> Co-authored-by: t-hamano <wildworks@git.wordpress.org> Co-authored-by: swissspidy <swissspidy@git.wordpress.org> Co-authored-by: kjnanda <krupajnanda@git.wordpress.org> Co-authored-by: matiasbenedetto <mmaattiiaass@git.wordpress.org>
pento
pushed a commit
to WordPress/wordpress-develop
that referenced
this pull request
Mar 28, 2024
This merges several high priority bug fixes for the editor ahead of WordPress 6.5: - WordPress/gutenberg#60180 - WordPress/gutenberg#60093 - WordPress/gutenberg#60071 - WordPress/gutenberg#60130 - WordPress/gutenberg#59959 - WordPress/gutenberg#60167 Props youknowriad, annezazu, mcsf, jsnajdr, mmaattiiaass, get_dave, scruffian, mikachan, grantmkin, andraganescu, scruffian, antosguillamot, fabiankaegy, huzaifaalmesbah, krupajnanda, colorful-tones, liviopv, mamaduka, kim88, poena, peterwilsoncc, wildworks, swissspidy, desrosj, jorbin. Fixes #60315. git-svn-id: https://develop.svn.wordpress.org/trunk@57888 602fd350-edb4-49c9-b593-d223f7449a82
markjaquith
pushed a commit
to markjaquith/WordPress
that referenced
this pull request
Mar 28, 2024
This merges several high priority bug fixes for the editor ahead of WordPress 6.5: - WordPress/gutenberg#60180 - WordPress/gutenberg#60093 - WordPress/gutenberg#60071 - WordPress/gutenberg#60130 - WordPress/gutenberg#59959 - WordPress/gutenberg#60167 Props youknowriad, annezazu, mcsf, jsnajdr, mmaattiiaass, get_dave, scruffian, mikachan, grantmkin, andraganescu, scruffian, antosguillamot, fabiankaegy, huzaifaalmesbah, krupajnanda, colorful-tones, liviopv, mamaduka, kim88, poena, peterwilsoncc, wildworks, swissspidy, desrosj, jorbin. Fixes #60315. Built from https://develop.svn.wordpress.org/trunk@57888 git-svn-id: http://core.svn.wordpress.org/trunk@57389 1a063a9b-81f0-0310-95a4-ce76da25c4cd
github-actions bot
pushed a commit
to gilzow/wordpress-performance
that referenced
this pull request
Mar 28, 2024
This merges several high priority bug fixes for the editor ahead of WordPress 6.5: - WordPress/gutenberg#60180 - WordPress/gutenberg#60093 - WordPress/gutenberg#60071 - WordPress/gutenberg#60130 - WordPress/gutenberg#59959 - WordPress/gutenberg#60167 Props youknowriad, annezazu, mcsf, jsnajdr, mmaattiiaass, get_dave, scruffian, mikachan, grantmkin, andraganescu, scruffian, antosguillamot, fabiankaegy, huzaifaalmesbah, krupajnanda, colorful-tones, liviopv, mamaduka, kim88, poena, peterwilsoncc, wildworks, swissspidy, desrosj, jorbin. Fixes #60315. Built from https://develop.svn.wordpress.org/trunk@57888 git-svn-id: https://core.svn.wordpress.org/trunk@57389 1a063a9b-81f0-0310-95a4-ce76da25c4cd
pento
pushed a commit
to WordPress/wordpress-develop
that referenced
this pull request
Mar 28, 2024
This merges several high priority bug fixes for the editor ahead of WordPress 6.5: - WordPress/gutenberg#60180 - WordPress/gutenberg#60093 - WordPress/gutenberg#60071 - WordPress/gutenberg#60130 - WordPress/gutenberg#59959 - WordPress/gutenberg#60167 Reviewed by jorbin, swissspidy. Merges [57888] to the 6.5 branch. Props youknowriad, annezazu, mcsf, jsnajdr, mmaattiiaass, get_dave, scruffian, mikachan, grantmkin, andraganescu, scruffian, antosguillamot, fabiankaegy, huzaifaalmesbah, krupajnanda, colorful-tones, liviopv, mamaduka, kim88, poena, peterwilsoncc, wildworks, swissspidy, desrosj, jorbin. Fixes #60315. git-svn-id: https://develop.svn.wordpress.org/branches/6.5@57891 602fd350-edb4-49c9-b593-d223f7449a82
markjaquith
pushed a commit
to markjaquith/WordPress
that referenced
this pull request
Mar 28, 2024
This merges several high priority bug fixes for the editor ahead of WordPress 6.5: - WordPress/gutenberg#60180 - WordPress/gutenberg#60093 - WordPress/gutenberg#60071 - WordPress/gutenberg#60130 - WordPress/gutenberg#59959 - WordPress/gutenberg#60167 Reviewed by jorbin, swissspidy. Merges [57888] to the 6.5 branch. Props youknowriad, annezazu, mcsf, jsnajdr, mmaattiiaass, get_dave, scruffian, mikachan, grantmkin, andraganescu, scruffian, antosguillamot, fabiankaegy, huzaifaalmesbah, krupajnanda, colorful-tones, liviopv, mamaduka, kim88, poena, peterwilsoncc, wildworks, swissspidy, desrosj, jorbin. Fixes #60315. Built from https://develop.svn.wordpress.org/branches/6.5@57891 git-svn-id: http://core.svn.wordpress.org/branches/6.5@57392 1a063a9b-81f0-0310-95a4-ce76da25c4cd
cbravobernal
pushed a commit
to garridinsi/gutenberg
that referenced
this pull request
Apr 9, 2024
…WordPress#60180) * Font Library: install fonts in sequence to work around font directory race condition * Update comment block to coding standards Co-authored-by: Peter Wilson <519727+peterwilsoncc@users.noreply.github.com> * Fix linting --------- Co-authored-by: creativecoder <grantmkin@git.wordpress.org> Co-authored-by: getdave <get_dave@git.wordpress.org> Co-authored-by: mcsf <mcsf@git.wordpress.org> Co-authored-by: peterwilsoncc <peterwilsoncc@git.wordpress.org> Co-authored-by: mikachan <mikachan@git.wordpress.org> Co-authored-by: huzaifaalmesbah <huzaifaalmesbah@git.wordpress.org> Co-authored-by: t-hamano <wildworks@git.wordpress.org> Co-authored-by: swissspidy <swissspidy@git.wordpress.org> Co-authored-by: kjnanda <krupajnanda@git.wordpress.org> Co-authored-by: matiasbenedetto <mmaattiiaass@git.wordpress.org>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What?
Install new font faces sequentially, rather than in parallel, to prevent a race condition where font installation errors because two requests try to create the font directory at the same time.
Closes #59023
Why?
Prevents font installation from partially erroring when the fonts directory doesn't exist yet (such as on new installs of WordPress or those updating to WP 6.5).
How?
Run the font-face creation requests in sequence using a for loop rather than
Promise.allSettled.Testing Instructions
Note that the race condition error can be difficult to replicate. There are specific instructions for that here: #59023 (comment)