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

WordPress Trac noreply at wordpress.org
Mon Jul 8 17:21:06 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 am not concerned about bypassing the uploads_dir filter in Core. I am
 concerned about introducing an entirely knew API to allow themes and
 plugins to do so.

 > My understanding of the PR is that it modifies the uploads API by adding
 a new parameter to wp_upload_bits() so the directory where files will be
 uploaded is directly passed to the function rather than applied as a
 filter.

 I see. So basically you are against themes and plugins being able to use
 `wp_upload_bits()` "directly" and having to do it "indirectly" by adding a
 callback to the `upload_dir` filter. Well, you're right that this sounds
 like an enhancement. Three months ago when this patch was created, this
 seemed like a good idea.

 Not sure if this was your intention but I agree that `wp_upload_bits()`
 has nothing to do with how fonts are uploaded in WP. Going to remove that
 part and maybe open an "enhancement" ticket with that idea. Please feel
 free to comment about it there.

 > Please refer to the coding standards on [https://developer.wordpress.org
 /coding-standards/wordpress-coding-standards/php/#closures-anonymous-
 functions closures]: "Closures should not be passed as filter or action
 callbacks, as removing these via remove_action() / remove_filter() is
 complex"
 >
 > This is why it becomes a maintenance burden. Core contributors unaware
 that the decision to go against the coding standards was intentional in an
 attempt to avoid an infinite loop when ''using the filter as documented''
 may reintroduce the bug.

 Ah, right, my bad. This made it in (was pushing for that change for
 years..). So basically the fix that prevents plugins from removing the
 /fonts directory that was committed in [57740] was reverted in [57868] to
 fix a coding standards issue. Do you think this was a good decision:
 reverting an important fix instead of adding a temporary coding standards
 exception?

 Also, [57868] was committed to WP 6.5 as "temporary solution that needs
 fixing soon", right? I don't believe that the "core contributors would
 have been unaware" of it, especially with some more docs/inline comments
 :)

 (Just for the record: I believe that the changes in [57868] were totally
 unnecessary and all that was needed was a coding standards exception and a
 bit more explanations that the code there will be updated soon to remove
 that exception, and how to avoid causing an infinite loop until the fix.)

 So, again, the reason you are blocking the progress on the "promise to fix
 that code" as [https://github.com/WordPress/wordpress-
 develop/pull/6211#issuecomment-2011252700  made on the PR] for the [57868]
 commit by @youknowriad and @swissspidy are... What exactly?

 Can you openly and honestly say why it is sooo important that this code
 remains unfixed, and what will WP gain by keeping it unfixed?

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


More information about the wp-trac mailing list