Make WordPress Core

Opened 5 years ago

Closed 5 years ago

Last modified 5 years ago

#48421 closed enhancement (fixed)

Add $unfiltered param to wp_get_original_image_path() and wp_get_original_image_url() functions

Reported by: ianmjones's profile ianmjones Owned by: azaozz's profile azaozz
Milestone: 5.4 Priority: normal
Severity: normal Version: 5.3
Component: Media Keywords:
Focuses: Cc:

Description

Since #48302 there are now both wp_get_original_image_path() and wp_get_original_image_url() functions that take an attachment id and return the original large pre-scaled image path or URL.

However, unlike wp_get_attached_file(), neither take an $unfiltered param.

This is problematic for plugins wanting to work with the local path or URL for an original file but either it or another plugin already implements filters that alter the wp_get_attached_file() or wp_get_attachment_metadata() results that both of these wp_get_original_image_* functions use.

Also, as per the wp_get_attached_file() and wp_get_attachment_metadata() functions, it would be good if the $unfiltered param meant the respective wp_get_original_image_path and wp_get_original_image_url filters were short-cut out and not run if $unfiltered is true.

Attachments (1)

48421.diff (1009 bytes) - added by azaozz 5 years ago.

Download all attachments as: .zip

Change History (11)

#1 @azaozz
5 years ago

  • Milestone changed from Awaiting Review to 5.4

As far as I see the $unfiltered params are somewhat crude way to pass context to these functions and filters (they were added quite some time ago). Perhaps it can be passed-through in wp_get_original_image_path(), but may be better to add support for full "context" there instead?

There's also no $unfiltered in wp_get_attachment_url(). Don't think introducing that is a good idea. Currently if a plugin wishes so, the filters can be bypassed by hooking-in early. The same code/method would still work for the new wp_get_original_image_url().

This ticket was mentioned in Slack in #core-media by antpb. View the logs.


5 years ago

#3 follow-up: @joemcgill
5 years ago

  • Keywords reporter-feedback added

For wp_get_original_image_path() it might make sense to pass true as the second parameter to the get_attached_file() call when that is used as a fallback for the image path. Like @azaozz, I don't see a great way of bypassing filters when calling wp_get_original_image_url() though.

@ianmjones can you share a use case where this is currently tripping you up?

This ticket was mentioned in Slack in #core by david.baumwald. View the logs.


5 years ago

#5 @davidbaumwald
5 years ago

  • Milestone changed from 5.4 to Future Release

This ticket still needs reporter feedback, and with 5.4 Beta 1 landing tomorrow, this is being moved to Future Release. If any maintainer or committer feels this can be included in 5.4 or wishes to assume ownership during a specific cycle, feel free to update the milestone accordingly.

#6 in reply to: ↑ 3 @ianmjones
5 years ago

Replying to joemcgill:

For wp_get_original_image_path() it might make sense to pass true as the second parameter to the get_attached_file() call when that is used as a fallback for the image path. Like @azaozz, I don't see a great way of bypassing filters when calling wp_get_original_image_url() though.

@ianmjones can you share a use case where this is currently tripping you up?

Because our plugin offloads a Media Library item to a storage provider's bucket and then optionally removes the local files, any call to wp_get_original_image_path() will ultimately be filtered to return the StreamWrapper URL instead of local path if the file does not exist locally. If the file does exist locally then WP Offload Media returns that unfiltered version.

Under certain scenarios you need the unfiltered local path (and unencoded file name) no matter what, e.g. you're preparing to copy the file back to its local path, or a backup or image processing plugin wants to work with local versions only and will check the path returned anyway.

For get_attached_file() we have the $unfiltered param, but if we need to get the original image's local path and name, which may or may not be different from the current full size image in various ways, we have no ability to let WordPress tell us what it should be.

The request for similar unfiltered URL functions is for consistency, scenarios where the "canonical" URL is required for adding to headers etc, and for quick comparison of URLs in content (WP Offload Media tries to ensure WordPress saves the local URL in database content and filters it where necessary).

#7 @ianmjones
5 years ago

  • Keywords reporter-feedback removed

@azaozz
5 years ago

#8 @azaozz
5 years ago

Agree with @joemcgill and @ianmjones that wp_get_original_image_path() shouldn't be preventing use of $unfiltered when getting the attached file path.

In 48421.diff: pass through $unfiltered to get_attached_file().

#9 @azaozz
5 years ago

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

In 47274:

Media: Pass through the $unfiltered param to get_attached_file() in wp_get_original_image_path().

Props ianmjones, joemcgill, azaozz.
Fixes #48421.

#10 @azaozz
5 years ago

  • Milestone changed from Future Release to 5.4
Note: See TracTickets for help on using tickets.