WordPress.org

Make WordPress Core

Opened 6 years ago

Closed 5 years ago

#11976 closed defect (bug) (fixed)

wp_iframe() style inclusion when using objects/methods

Reported by: technosailor Owned by: nacin
Milestone: 3.0 Priority: normal
Severity: normal Version:
Component: Media Keywords: has-patch
Focuses: Cc:

Description

Small bug in wp_iframe() pertaining to the inclusion of the media stylesheet. The determination of whether to include the media stylesheet is based on the assumption that the callback passed to wp_iframe() is a function name and ignores the possibility of inclusion of a class/method array pair.

Attached patch corrects this problem.

Attachments (2)

11976.diff (591 bytes) - added by technosailor 6 years ago.
wordpress.diff (748 bytes) - added by vericgar 5 years ago.

Download all attachments as: .zip

Change History (10)

@technosailor6 years ago

comment:1 @nacin6 years ago

  • Component changed from General to Media
  • Keywords has-patch added; css wp-admin media removed
  • Milestone changed from Unassigned to 3.0

comment:2 @nacin6 years ago

Would probably make sense to rely on is_callable() for this.

comment:3 @filosofo6 years ago

It would probably be better to rewrite the logic of the function, which is a mess: Allowing an indefinite number and arrangement of arguments; assuming the function is a string in one place but checking whether it is in another. None of which is necessary since almost all of the media_* functions have a similar argument structure.

And this is a good example of how having PHP 5 would help make things more robust. You could make the media functions to be objects that implement a media interface. Type hinting in wp_iframe would cover the necessary check, and then you could use the sub-classed media object to enqueue scripts, provide class names, or whatever.

/rant

comment:4 @technosailor6 years ago

I think what you're expressing is having a generic media object that could be extended by a variety of extension classes. WordPress would come bundled with the class that extends the media object to create the default media upload interface but a different subclass could be provided via plugin, such as is done now when creating widgets with the new Widget API.

I don't think you need PHP 5 for that. But your point is well received that PHP 5 would make it easier. Unfortunately, I don't have the time to rewrite the whole damn function but wouldn't mind if it was because, you're right, it's a nasty mess. The patch I provided is low hanging fruit to get the bug, as it exists now, fixed up.

comment:5 @nacin6 years ago

  • Owner changed from technosailor to nacin
  • Status changed from new to accepted

comment:6 @nacin6 years ago

  • Resolution set to fixed
  • Status changed from accepted to closed

(In [13243]) Allow class/method array pair in wp_iframe() when checking callback name in determining whether to include media stylessheet. Props technosailor, fixes #11976

comment:7 @vericgar5 years ago

  • Cc vericgar added
  • Resolution fixed deleted
  • Status changed from closed to reopened

I just ran into this bug using Wordpress 3.0.1

The fix provided has a small logic error, and causes this message when WP_DEBUG is on:

Notice: Array to string conversion in [omitted]/wp-admin/includes/media.php on line 310

I will attach another patch.

@vericgar5 years ago

comment:8 @nacin5 years ago

  • Resolution set to fixed
  • Status changed from reopened to closed

(In [16729]) Fix notice. props vericgar, fixes #11976.

Note: See TracTickets for help on using tickets.