Make WordPress Core

Opened 9 years ago

Last modified 7 years ago

#34465 new defect (bug)

Uploader in Media Modal Not Working When Certain Library Arguments Present

Reported by: funkatronic's profile Funkatronic Owned by:
Milestone: Awaiting Review Priority: normal
Severity: normal Version: 4.0
Component: Media Keywords: has-screenshots needs-patch
Focuses: ui Cc:

Description

I noticed this bug while working on a plugin that deals with the media modal. I'm using the following code to create a media modal on a button view:

function frame(){
				// Destroy the previous collection frame.
				if ( this._frame )
				{
					this.stopListening( this._frame );
					this._frame.dispose();
				}
	
				this._frame = wp.media( {
					className: 'media-frame rwmb-media-frame',
					multiple : true,
					title    : 'Select Media',
					library  : {
						type   : 'image',
					}, 
					frame: 'select',
				} );

				//Event stuff goes here
	
				this._frame.open();
			},

The issue is when uploading a file using the upload tab in the media modal. The file uploads but it never appears in the Media Library tab like it does in the core media modal used by the editor. The side Attachment Details panel shows the file information but the file itself doesn't appear on the grid. Attached is a screen cap from a user of the plugin.

Attachments (1)

ce63611e-7be6-11e5-8b46-aee7a641ad14.png (438.0 KB) - added by Funkatronic 9 years ago.
A most heinous bug indeed

Download all attachments as: .zip

Change History (20)

@Funkatronic
9 years ago

A most heinous bug indeed

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


9 years ago

#2 @Funkatronic
9 years ago

  • Keywords has-screenshots added

#3 @Funkatronic
9 years ago

  • Keywords needs-testing added

Alright, found the cause of the problem but not sure of a solution. In my code, I'm using exclude to exclude certain files each time I open the media modal. Apparently using exclude prevents the browser part of the modal from being updated when I upload a file in the same modal. Definitely considered a bug in my book but I'm not sure how to resolve this.

The code to replicate the bug is such:

function frame(){
	var frame = wp.media( {
		className: 'media-frame rwmb-media-frame',
		multiple : true,
		title    : 'Select Media',
		library  : {
			type   : 'image',
			exclude: [] //put excluded file ids here
		}, 
		frame: 'select',
	} );

	this._frame.open();
};
Version 0, edited 9 years ago by Funkatronic (next)

#4 @Funkatronic
9 years ago

  • Summary changed from Uploading in Media Modal Not updating Media Library Tab to When Excluding Attachments, Media Modal Browser Doesn't Update on Uploads

#5 @wonderboymusic
9 years ago

  • Version changed from trunk to 4.0

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


9 years ago

#7 @Funkatronic
9 years ago

Bug also triggered if the library of the modal is filtered by author, as seen in this post in Slack: https://wordpress.slack.com/archives/core/p1449252012003031

#8 @nikolov.tmw
9 years ago

I found the root of the issue in Query.initialize(). If you look at the end of the function( https://core.trac.wordpress.org/browser/trunk/src/wp-includes/js/media-models.js#L1176 ), you'll see this:

// Observe the central `wp.Uploader.queue` collection to watch for
// new matches for the query.
//
// Only observe when a limited number of query args are set. There
// are no filters for other properties, so observing will result in
// false positives in those queries.
allowed = [ 's', 'order', 'orderby', 'posts_per_page', 'post_mime_type', 'post_parent' ];
if ( wp.Uploader && _( this.args ).chain().keys().difference( allowed ).isEmpty().value() ) {
	this.observe( wp.Uploader.queue );
}

If I add 'author' to the allowed array, it works as expected.

I understand why it was implemented(since there's only a handful of filters in this.filters defined), however I've been struggling to find a way to override this functionality. I've tried extending multiple classes in order to get past this limitation, but with no success so far(it could be just my poor OOP JS skills though).

I feel like there should be an easier way to accomplish the usage of additional query parameters, without having to extend many classes just to override a bit of functionality. Perhaps optional parameters to the wp.media() function?

On a side note - it's worth noting, that in order to implement additional query parameters, you also might have to hook to the 'ajax_query_attachments_args' filter, since only certain query params are allowed by default.

#9 @Funkatronic
9 years ago

@nikolov.tmw I'm trying to figure out the logic of that code. It should observe the uploader because you have type and therefore post_mime_type set but it doesn't. Weird. Thanks for the find.

#10 @nikolov.tmw
9 years ago

@Funkatronic From what I grasp, it only observes the uploader if all of your parameters match the allowed array. So, since I have 'author' in my parameters, the _( this.args ).chain().keys().difference( allowed ).isEmpty().value() part fails and no observing takes place.

The problem is that we have no easy way to extend/override the allowed array(short of subclassing all of the related classes just to get to the Query class). That's as far as I understand the code of course.

#11 @Funkatronic
9 years ago

Okay, now I understand what the code does, just not why it does it. If there are other arguments for the library query other than the allowed, it won't observe the Uploader queue, which results in the Uploader not updating the modal when a file is uploaded. I don't understand why this is there or why it is needed.

#12 @Funkatronic
9 years ago

  • Summary changed from When Excluding Attachments, Media Modal Browser Doesn't Update on Uploads to Uploader in Media Modal Not Working WhenNot Allowed Library Arguments Present

Uploader in Media Modal Not Working When Certain Library Arguments Present

Last edited 9 years ago by Funkatronic (previous) (diff)

#13 @Funkatronic
9 years ago

  • Summary changed from Uploader in Media Modal Not Working WhenNot Allowed Library Arguments Present to Uploader in Media Modal Not Working When Certain Library Arguments Present

#14 @Funkatronic
9 years ago

Okay, so I'm rereading the comment for the line of code and it says that is "Only observe when a limited number of query args are set", but what it is actually doing is only observing if only those args are set; if there are other args set, it won't observe. I don't think the code is working as intended and if it is I don't know what its doing or problem its trying to solve.

Last edited 9 years ago by Funkatronic (previous) (diff)

#15 @nikolov.tmw
9 years ago

I'd go on a limb here and say that it's done, so that in case you have multiple modal instances(say multiple uploaders on the same page), each one can have different query parameters.

However the uploader queue is only one and so all modals would listen for when new attachments are uploaded globally and have to validate that the new attachment belongs to their collection of attachments(according to the query parameters).

In the case of author in particular, it's really unlikely that you'd get a new upload that would not belong to that collection, however picture this situation(which is currently handled, but imagine it was not):

In one modal, you're allowing the user to create a featured gallery for the post. They can only select attachments that are uploaded to the current post. On the same page, you're also allowing the user to select a photo of the author that wrote the article(imagine guest blogging and you don't want to rely on the guest blogger's Gravatar).

Since this author might write multiple articles on your blog, you let the user select from all attachments(so they don't have to upload a new one every time).

Now, if I uploaded a new image to the modal that shows me all attachments and we didn't have the uploadedTo validation function(line 918), this new attachment could incorrectly show under the modal that only shows me the current post's attachments(if the global modal is configured to only upload globally - is that possible?).

Might not be the best example, but I think it should illustrate what problem they are solving with that code - validating new attachments to make sure that they fit the restrictions put on each collection.

#16 @Funkatronic
9 years ago

It's not validating, though; it's just ignoring the uploader if there is a arg it doesn't want to handle. What should happen is a callback on the queue update that makes sure the new file fits with the arguments of that modal's library, then add it in. This code just gives a blanket "no, I don't want to handle that" that breaks modals that want to do something different.

#17 @Funkatronic
9 years ago

  • Keywords needs-patch added; needs-testing removed

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


9 years ago

#19 @barryceelen
7 years ago

A quick fix for this problem might be explicitly telling your frame's library to observe the wp.Uploader.queue.

Using the example by @Funkatronic:

frame.states.get('library').get('library').observe( wp.Uploader.queue );

Note that this disregards any possible implications mentioned by @nikolov.tmw.

Note: See TracTickets for help on using tickets.