[#44848] - [6.0] Media: fix large file upload by using of binary upload instead of base64, and display correct upload progress
- Pending
- Medium
- Build: 5.3-dev
- # 44848
- Diff
- Fedik:media-blb-upload
User tests: Successful: Unsuccessful:
Summary of Changes
The PR changes the media manager to use FormData for file upload, instead of base64 payload.
Which improves responsiveness, performance and handling of large files in general.
However the api/ calls still uses base64 because it is hard to do in b/c maner.
Bonus: the upload progres now showing actual upload progress.
Testing Instructions
Apply patch.
Run npm install.
Go to media manager and try upload something bigger than 100Mb
Addittionaly, test with any 3d filesystem plugin.
Actual result BEFORE applying this Pull Request
The browser hangs for a while, and dependent from your luck the upload will be completed after some waiting.
Expected result AFTER applying this Pull Request
The upload starts immediately, and you can see progress (a litle 5px blue line over the files list)
Link to documentations
Please select:
- Documentation link for docs.joomla.org:
- No documentation changes for docs.joomla.org needed
- Pull Request link for manual.joomla.org: joomla/Manual#431
- No documentation changes for manual.joomla.org needed
| Status | New | ⇒ | Pending |
joomla-cms-bot
- | Category | ⇒ | JavaScript Administration com_media NPM Change Repository Front End Plugins |
| Labels |
Added:
Feature
NPM Resource Changed
PR-5.3-dev
Performance
|
||
| Title |
|
||||||
ApiController
Hmhm,
I would read the file content it in the api controller and then pass it to the adapter the same way as before that no changes are needed in the adapter
No no, this not going to work, you will get "out of memory" error when try to read 100Mb file when PHP limit is 60Mb.
This will bring back problem with large file upload.
Loading whole file in to memory should never happen while uploading (unless when it realy need).
What about stringable object?
We wrap uploaded file in to that object, and the file will be loaded in to memory only when the plugin expecting a string. Addittionaly we deprecate this behavior.
What do you think?
Not sure if stringable would solve it. But I think the main goal should be to do it in a bc way.
| Category | JavaScript Administration com_media NPM Change Repository Front End Plugins | ⇒ | JavaScript Administration com_media NPM Change Repository Libraries Front End Plugins |
I updated the code to use stringable object.
In my tests it works well even when try to access the data as to the string.
Would be good to have some more test, to be sure, with 3rd filesystem adapters.
I have not tested this item.
Pathch not applied. Got error The file marked for modification does not exist: administrator/components/com_media/resources/scripts/app/Api.es6.js while applying patch
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/44848.
I have not tested this item.
Got error while applying patch.The file marked for modification does not exist: administrator/components/com_media/resources/scripts/app/Api.es6.js
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/44848.
@Tejashrimajage how do you apply the patch ?
@Tejashrimajage how do you apply the patch ?
@alikon I am using patch tester component for appling a patch.
This cannot be tested with patch tester
as this pull request is adding/changing some stuff involving js/node/vue etc , maybe better if you test with prebuild package https://artifacts.joomla.org/drone/joomla/joomla-cms/5.3-dev/44848/downloads/82047/
This pull request has been automatically rebased to 6.0-dev.
| Title |
|
||||||
| Category | JavaScript Administration com_media NPM Change Repository Front End Plugins Libraries | ⇒ | Administration com_associations com_banners com_categories com_checkin com_config com_contact com_content com_contenthistory com_cpanel com_fields com_installer com_joomlaupdate JavaScript com_media NPM Change com_menus com_modules |
| Labels |
Added:
PR-6.0-dev
Removed: PR-5.3-dev |
||
| Category | JavaScript Administration com_media NPM Change com_associations com_banners com_categories com_checkin com_config com_contact com_content com_contenthistory com_cpanel com_fields com_installer com_joomlaupdate com_menus com_modules | ⇒ | JavaScript Administration com_media NPM Change Repository Libraries Front End Plugins |
| Category | JavaScript Administration com_media NPM Change Repository Libraries Front End Plugins | ⇒ | JavaScript Administration com_media NPM Change Repository Front End Plugins |
I like that change, but this will break 3rd party extensions when the data can be all of the sudden a resource. I would read the file content it in the api controller and then pass it to the adapter the same way as before that no changes are needed in the adapter. Additionally, the check content logic can probably be moved to the, but not sure about this.