WordPress.org

Make WordPress Core

Opened 2 years ago

Closed 2 years ago

Last modified 2 years ago

#20532 closed defect (bug) (fixed)

Cannot use more than one wp.Uploader (Plupload) instance

Reported by: kovshenin Owned by: koopersmith
Milestone: 3.4 Priority: normal
Severity: normal Version: 3.4
Component: Upload Keywords:
Focuses: Cc:

Description

Hey there!

Found this bug while playing with the Customizer's Image control and the image context. Turns out every control will use the same Plupload instance, so the context will be set to the one, specified by the latest control rendered on screen. This verifies that no matter how many "new" wp.Uploader objects you create, it is still the same one:

var u1 = new wp.Uploader();
var u2 = new wp.Uploader();

u1.param( 'some_key', 'first value' );
u2.param( 'some_key', 'second value' );

console.log( u1.uploader.settings.multipart_params.some_key ); // logs: second value
console.log( u2.uploader.settings.multipart_params.some_key ); // logs: second value

I'm not too confident in my JS skills to handle a patch, sorry.

Change History (5)

comment:1 koopersmith2 years ago

  • Milestone changed from Awaiting Review to 3.4
  • Owner set to koopersmith
  • Status changed from new to accepted

Pretty sure I know what's happening here. Thanks for the report.

comment:2 koopersmith2 years ago

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

In [20577]:

Use a deep extend in wp.Uploader to ensure that the multipart_params object (and others) are cloned instead of referenced. fixes #20532.

comment:3 follow-up: azaozz2 years ago

Shouldn't this be:

var object = $.extend({}, object1, object2);

so object1 and object2 are copied instead?

Version 0, edited 2 years ago by azaozz (next)

comment:4 kovshenin2 years ago

The fix in [20577] worked for me. My image and upload fields in the Customizer now come with the correct context. Thanks!

comment:5 in reply to: ↑ 3 koopersmith2 years ago

Replying to azaozz:

Shouldn't this be:

var object = $.extend({}, object1, object2);

so both object1 and object2 are copied instead?

In this case, since object1 is a new object, there's no need to copy it into a blank object. What we needed was a deep copy, which is why we set the first parameter to true. If we didn't use deep copy, any default key that in the settings that referred to an object (such as the multipart_params key) would be copied by reference. Since there is only one default object, each instance would end up changing the defaults when trying to change the settings on the instance.

Note: See TracTickets for help on using tickets.