Repository navigation
Keep font files another site uses when deleting or editing a custom font - #2012
Merged
Merged
Conversation
Contributor
|
Coverage report for commit: 1f2b54c Summary - Lines: 93.99% | Methods: 90.96% | Branches: 85.58%
🤖 Jest coverage report |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Contributor
|
Coverage report for commit: 1f2b54c Summary - Lines: 90.26% | Methods: 95.89%
🤖 PHPUnit coverage report |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
4 of 6 tasks
gravitypdf-bot
force-pushed
the
fix/multisite-safe-font-crud
branch
from
November 6, 2026 00:29
144267d to
8ecea65
Compare
gravitypdf-bot
changed the base branch from
feature/core-font-upgrade-prune
to
feature/font-file-conflict-rename
November 6, 2026 00:29
jakejackson1
force-pushed
the
feature/font-file-conflict-rename
branch
from
November 6, 2026 03:06
f7acf3e to
74c3b06
Compare
5 of 9 tasks
gravitypdf-bot
force-pushed
the
feature/font-file-conflict-rename
branch
3 times, most recently
from
November 9, 2026 06:09
62cbe50 to
38e02af
Compare
Base automatically changed from
feature/font-file-conflict-rename
to
development
November 9, 2026 06:12
On multisite the fonts folder is network-wide but each site keeps its own custom_fonts records, and sites can point at the same file (records from before 6.18.0's unique upload names, and premium templates' add_pdf_font() copies). Deleting a font, or replacing or removing one of its styles, unlinked the file without checking the other sites, breaking their PDFs. This has been the case since 6.0. Model_Custom_Fonts::get_files_used_by_other_sites() lists the basenames every other site's records use. It reads each site's gfpdf_settings straight from that site's options table rather than switching blogs, skips corrupt values, and is memoised per request. It returns nothing on single site and runs no queries there. Controller_Custom_Fonts' delete_item() and update_item() now remove files through one method that keeps any file on that list (logged at debug level), and skips the lookup when there is nothing to delete. Matching by basename errs towards keeping a file: the worst case is an orphan, never a broken PDF. Model_Font_Sources::remove_record() can then always go through GPDFAPI::delete_pdf_font(), dropping the multisite branch that only removed the record and left every file behind. Adoption now also removes a file on multisite once no other site uses it. The style keys move to Model_Custom_Fonts::FONT_STYLES so the controller, font sources and the lookup share one list. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
gravitypdf-bot
force-pushed
the
fix/multisite-safe-font-crud
branch
from
November 10, 2026 05:00
8ecea65 to
1f2b54c
Compare
This file contains hidden or 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
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.
Summary
On multisite, a site admin deleting a custom font, or replacing or removing one of its styles, could delete a font file that another site's font still uses. That broke the other site's PDFs, because the fonts folder is shared across the network while each site keeps its own font records. This has been the case since 6.0. Font Manager now keeps a file when another site's font still points at it, and the 6.19 font source adoption goes back to removing files through the normal delete path on multisite.
Model_Custom_Fonts::get_files_used_by_other_sites()lists the file names other sites' fonts use. On single site it returns nothing and runs no queries.Controller_Custom_Fonts::delete_item()andupdate_item()now delete files through onedelete_unused_font_files(), which keeps a file another site uses.Model_Font_Sources::remove_record()always callsGPDFAPI::delete_pdf_font(). Its multisite branch, which removed only the record and left every file behind, is gone.Model_Custom_Fonts::FONT_STYLES.Try it
custom_fontsentry into site B'sgfpdf_settings..ttffile stays inwp-content/uploads/PDF_EXTENDED_TEMPLATES/fonts/, and site B's PDFs still render in it.Test plan
@group ms-requiredtests: deleting on one site keeps the shared file and the other site's record, and deleting on the last site removes it;update_item()replacing a shared style keeps the old file and deletes a file only this site used; corruptgfpdf_settingson other sites doesn't fatal; the lookup is memoised;remove_record()adoption on multisite removes the record and keeps a shared file@group ms-excluded): the lookup returns[]with no queriestest_an_unwritable_folder_fails_without_a_request×2 andtest_the_kill_switch_constant_turns_downloads_off: the container runs as root, plus a harness constant warning)More info
switch_to_blog()), skips corrupt values, and is memoised per request. It stays inModel_Custom_Fonts, which owns the records it reads.get_sites()plus one options query per other site, only on an admin delete, an edit that drops a file, or a font source adoption that removes a record. It never runs on render. The lookup is skipped when there's nothing to delete, such as a rename or an OTL/Kashida change.switch_to_blog()in the same request reads fresh data.Model_Custom_Fonts::flush()clears it, andCopiesFontFixtures::remove_font_fixtures()calls it so tests don't leak state through the singleton.upload_with_unique_name()never overwrites an existing file.Model_Font_Downloader::prune()keeps the fonts every site needs, but resolves their keys through the cron site's font list. A custom font only another site has resolves to no files, so a record pointing at an unmodified core font file under a key adoption doesn't match isn't protected.