Make WordPress Core

Opened 3 weeks ago

Last modified 10 days ago

#61297 reopened defect (bug)

Font Directory uploads ignore `subdir` property.

Reported by: peterwilsoncc's profile peterwilsoncc Owned by: peterwilsoncc's profile peterwilsoncc
Milestone: 6.5.5 Priority: normal
Severity: normal Version:
Component: Editor Keywords: has-patch has-unit-tests dev-feedback
Focuses: Cc:

Description (last modified by peterwilsoncc)

The data structure for the font directory matches the uploads directory and contains the following properties:

  • path
  • url
  • subdir
  • basedir
  • baseurl
  • error (only used during directory creation)

When uploading fonts, the sub-directory is not stored in the post meta and prevents the deletion of files when the font-face post objects are deleted.

add_filter( 'font_dir', function ( $font_dir ) {
        /*
         * Change sub-directory every 5 minutes.
         *
         * POC only, a more likely use is to get the `fontFamily` from the API
         * request or the currently active theme and use either of those values
         * to generate a unique sub-directory.
         *
         * The POC is simply to allow for eaiser testing by setting a new
         * sub-directory every few minutes.
         */
        $subdir = '/' . (string) floor( time() / 300 );
        $font_dir['subdir'] = $subdir;
        $font_dir['path'] .= $subdir;
        $font_dir['url'] .= $subdir;
        return $font_dir;
} );

The following will need to be updated to account for the sub-directory:

  • post meta data containing the file reference
  • deletion of files in _wp_before_delete_font_face()

To reproduce:

  • Upload a font/sideload from Google
  • Observe the font file is stored in a sub-directory
  • Observe the font file's sub-directory is not stored in the post meta
  • Wait five minutes (using sample plugin above)
  • Delete the font via the font library
  • Observe the file is not deleted

Change History (9)

#1 @peterwilsoncc
3 weeks ago

  • Description modified (diff)

#2 @peterwilsoncc
3 weeks ago

Edited the description to clarify the bug: the sub-directory is not stored in the post meta.

This ticket was mentioned in PR #6638 on WordPress/wordpress-develop by @peterwilsoncc.


3 weeks ago
#3

  • Keywords has-patch has-unit-tests added

@peterwilsoncc commented on PR #6638:


3 weeks ago
#4

@costdev Done, the failing tests can be seen against 134770eb5d6ba43faa1fb5475890e188ec890a77 and passing against the latest commit.

The Gutenberg tests are failing but it looks to be typescript related so I am not sure what is happening there.

#5 @peterwilsoncc
3 weeks ago

  • Milestone changed from Awaiting Review to 6.6

Moving this to the 6.6 milestone while a discussion takes place in Slack as to whether it ought to be included in a minor 6.5 release.

#6 @grantmkin
10 days ago

I've tested and approved the PR attached to this ticket (https://github.com/WordPress/wordpress-develop/pull/6638#pullrequestreview-2099643069). Thanks for finding and fixing this @peterwilsoncc !

#7 @peterwilsoncc
10 days ago

  • Owner set to peterwilsoncc
  • Resolution set to fixed
  • Status changed from new to closed

In 58353:

Editor (Font Library): Store font subdirectory in post meta.

Stores the font file sub-directory in the wp_font_face post meta. Similar to attachments, only the portion of the path relative to the base directory is stored.

This ensures the files can be deleted alongside their post on sites using a plugin to store font files in sub-directories. Previously running such a plugin would result in the files remaining on the file system post delete.

Props costdev, grantmkin, peterwilsoncc.
Fixes #61297.

#8 @peterwilsoncc
10 days ago

  • Keywords dev-feedback added
  • Milestone changed from 6.6 to 6.5.5
  • Resolution fixed deleted
  • Status changed from closed to reopened

Re-opening and moving to the minor release milestone for backport consideration.

If it's decided not to backport, please close as fixed and move the ticket back to WP 6.6.

Note: See TracTickets for help on using tickets.