WordPress.org

Make WordPress Core

Opened 4 years ago

Closed 4 years ago

Last modified 3 years ago

#25663 closed task (blessed) (fixed)

Upgrade Plupload to 2.0

Reported by: azaozz Owned by:
Milestone: 3.9 Priority: normal
Severity: normal Version:
Component: External Libraries Keywords: needs-testing has-patch
Focuses: Cc:

Description

Plupload 2.0 was released about a month ago. Has quite a bit of fixes and improvements. The upgrade should be a straight replacement.

Attachments (4)

25663.patch (187.1 KB) - added by azaozz 4 years ago.
25663.diff (271.0 KB) - added by kovshenin 4 years ago.
25663.2.diff (2.7 KB) - added by kovshenin 4 years ago.
25663.3.diff (2.2 KB) - added by azaozz 4 years ago.

Download all attachments as: .zip

Change History (21)

#1 @azaozz
4 years ago

Consider removing the Silverlight component. It is useful only for IE < 10 when there's no flash, which seems quite rare.

#2 @dd32
4 years ago

  • Component changed from Upload to External Libraries

#3 @ocean90
4 years ago

  • Keywords needs-patch early added
  • Milestone changed from 3.8 to Future Release

Since the current uploader isn't much broken we can punt this for now.

Just an info: These are the files of 2.0.0:
moxie.js
moxie.min.js
Moxie.swf
Moxie.xap
plupload.dev.js
plupload.full.min.js
plupload.min.js
license.txt

And these are the current ones from core:
changelog.txt
license.txt
plupload.flash.js
plupload.flash.swf
plupload.html4.js
plupload.html5.js
plupload.js
plupload.silverlight.js
plupload.silverlight.xap

@azaozz
4 years ago

#4 @azaozz
4 years ago

  • Keywords needs-testing added; needs-patch early removed
  • Milestone changed from Future Release to 3.9

In 25663.patch:

  • Plupload 2.1.1.
  • Remove support for Silverlight. Flash is used in IE < 10. If flash is not installed, it will fall back to html4.
  • Remove the limitation and warnings for the flash runtime when uploading files larger than 100MB. Works well now.
Last edited 4 years ago by azaozz (previous) (diff)

@kovshenin
4 years ago

#5 @kovshenin
4 years ago

  • Keywords has-patch added

Refreshed in 25663.diff, added Silverlight back together with Moxie.xap. The patch contains binary files as well as file deletions, should apply cleanly with svn patch path/to/file.diff.

Tested the html5, flash and html4 runtimes in Chrome on OS X, looks good. The flash runtime doesn't work in Safari and Firefox, still trying to debug why. Haven't tested Windows, IE or the silverlight runtime yet. Any help appreciated.

#6 @azaozz
4 years ago

In 27316:

Upgrade Plupload to 2.1.1, props kovshenin, see #25663

#7 @azaozz
4 years ago

In 27318:

Remove debugging cruft, see #25663

#8 @TobiasBg
4 years ago

The moxie.xap has svn:executable set, so that file rights change from 644 to 755.
Not sure if this is necessary? Could that maybe cause problems with automatic updates?

#9 @azaozz
4 years ago

In 27340:

Plupload: remove the (old) plupload.silverlight.xap, remove the "executable" prop from Moxie.xap, see #25663

#10 @nacin
4 years ago

  • Type changed from enhancement to task (blessed)

@kovshenin
4 years ago

#11 follow-up: @kovshenin
4 years ago

Looked deeper into the Flash runtime issue in Firefox, Safari. It looks like Moxie.swf doesn't pass any cookies to async-upload.php so that's responding with a redirect. It works fine in Chrome and IE.

Been digging around the Plupload/Moxie forums and repo, but couldn't find much related. Closest I got was this 4yr old issue, where a suggested workaround was to include the session hash in the request. But then again everything worked well in 1.5.7 so I'm guessing it's a bug upstream with the change to Moxie.swf.

To work around this we can partially revert r18674 where we stopped passing our authentication cookies to post_params. The checks for them are still in place in async-upload.php, but need a small tweak to support the upload-attachment action (background: r22902).

Patch in 25663.2.diff.

@azaozz
4 years ago

#12 in reply to: ↑ 11 @azaozz
4 years ago

Replying to kovshenin:

To work around this we can partially revert r18674 where we stopped passing our authentication cookies to post_params.

Not sure this is a good idea. Our cookies are HTTP only, exposing them in the HTML is not good. That was one of the reasons for switching to Plupload too.

Emailed Davit (the lead developer of Plupload) and he traced the problem to the removal of the urlstream_upload option from 2.0. This was setting alternate mode in the flash runtime which ensures sending of cookies but requires the whole file to be loaded in memory. This also prevents uploading of big files with flash.

The same can be done in 2.0 by using

required_features: {
     send_binary_string: true
}

Tested further and it seems flash always sends cookies in IE, regardless of the above setting. 25663.3.diff is based on that. It checks whether flash will be used, then sets the above option in non IE browsers. As by default we use flash only for old IE, this seems the best alternative.

More testing especially in IE < 10 very welcome :)

#13 @azaozz
4 years ago

In [27662]:

Plupload: switch to urlstream upload method when the 'flash' runtime is used in non IE browsers. This ensures cookies are sent but limits the maximum file size that flash can handle.

By default only IE9 and older use flash so this change is mostly for completeness. It would affect only the (extremely rare) cases where a plugin disables the html5 runtime.

#14 @azaozz
4 years ago

[27662] changes the runtime order to html5,flash,silverlight,html4 which is the default in Plupload. That used to be html5,silverlight,flash,html4 as flash has problems with big files when using 'urlstream_upload' or send_binary_string: true. Without that setting (currently default for IE) flash seems to perform better than silverlight.

Lets test this for few days and revert it if there are any problems. Testing big files (over 200MB) in old IE would be best.

Last edited 4 years ago by azaozz (previous) (diff)

#15 @kovshenin
4 years ago

I tested IE9 and IE8 with Flash and a 500MB file and it worked like a charm. Like a slow charm.

#16 @nacin
4 years ago

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

Either re-open this ticket or create new tickets for additional issues.

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


3 years ago

Note: See TracTickets for help on using tickets.