[wp-trac] [WordPress Trac] #60835: Fix and improve handling of uploading of font files

WordPress Trac noreply at wordpress.org
Mon Jul 8 18:05:26 UTC 2024


#60835: Fix and improve handling of uploading of font files
----------------------------------------------------+---------------------
 Reporter:  azaozz                                  |       Owner:  (none)
     Type:  defect (bug)                            |      Status:  new
 Priority:  high                                    |   Milestone:  6.6
Component:  Upload                                  |     Version:  6.5
 Severity:  normal                                  |  Resolution:
 Keywords:  needs-testing has-patch has-unit-tests  |     Focuses:
----------------------------------------------------+---------------------

Comment (by azaozz):

 Replying to [comment:71 peterwilsoncc]:

 > I don't think that [attachment:"60835.diff"] is appropriate code to
 introduce during the release candidate phase.

 Why not? It does not change the way that code works at the moment. It
 actually doesn't change absolutely anything except to prevent plugins from
 removing the font_dir filter and the /fonts directory.

 Do you think the changes in 60835.diff are more "inappropriate" than the
 changes in [57868]? Could you please review 60835.diff and if you find it
 "inappropriate" share here what exactly is inappropriate about it?

 Btw, have you actually considered what would happen if a plugin removes
 the /fonts directory? This will alter the WP installation of the users of
 such plugin, most likely without them realizing it. The removal of /fonts
 would make it impossible for the users to use other plugins that expect
 that a /fonts directory exists. It will also make it impossible for the
 users to use future enhancements that require a /fonts directory, like for
 example being able to install fonts the same way as themes and plugins,
 and being able to manage installed fonts with git/svn.

 Have you also considered what will happen to the users if they install
 such plugin after they have installed few fonts? Then the previously
 installed fonts will be in (now dysfunctional) /fonts directory, and the
 newly installed fonts will be dumped in the /year/month sub-directories in
 /uploads.

 What about the opposite case: a user deactivates a plugin that removes the
 /fonts directory. Now they end up with previously installed fonts that are
 in few /year/month sub-directories and new fonts in a /fonts directory.

 Plugin that remove the /fonts directory are by default incompatible with
 plugins that use it, and with future core enhancements.

 Do you really believe this is a good way for WordPress to work? I believe
 this is a terrible way...

 > This ticket has had multiple committers say the proposed API is a
 problem

 Ummm, this is NOT true :) One committer (you) has said everything possible
 to delay/block/prevent/derail/discourage fixing of this code. One
 committer, @joemcgill has asked a question that I just answered above, and
 one contributor, @grantmkin (@creativecoder on GH) who is one of the
 authors of the original code, has said this looks good.

 > I think the best thing you could do for now is move this ticket off the
 6.6 milestone and spend some time reviewing feedback and reconsidering the
 approach. I've agreed to be co-tech lead on 6.7 and am willing to continue
 working towards a solution that doesn't introduce such a big change to the
 upload APIs.

 Thanks for your advice! Just wondering, would you follow this advice if
 you were in my place?

 I.e.
 - Some no-good code was committed few days before the WP 6.5 release. It
 is no-good as all four reviews for it are negative, and was committed as
 "temporary patch that needs fixing".
 - You attempt to fix it, but then the committer that wrote the no-good
 code and committed it starts to block any attempts for a fix with all
 means possible, and I mean all means!
 - Eventually the patch is delayed soooo much as to make it impossible to
 commit to WP 6.6.
 - You come up with an alternative patch that doesn't affect anything else
 just fixes the worse bug that was re-introduced in the 6.5 commit: plugins
 removing a WP directory.
 - And finally the committer that wrote the no-good code, then committed it
 to 6.5 after 4 negative reviews, and then blocked all attempts to fix it
 gives you "advice" to just forget fixing it now, without even reviewing
 the latest patch?

 Not sure about you but I think this is a really bad advice... Basically
 you're saying "do not attempt to make WP better!", isn't it?

-- 
Ticket URL: <https://core.trac.wordpress.org/ticket/60835#comment:73>
WordPress Trac <https://core.trac.wordpress.org/>
WordPress publishing platform


More information about the wp-trac mailing list