WordPress.org

Make WordPress Core

Opened 23 months ago

Last modified 21 months ago

#42794 new defect (bug)

Can't upload new media files when multiple media dialogues are present on a single page

Reported by: carlobeltrame Owned by:
Milestone: Awaiting Review Priority: normal
Severity: normal Version: 3.7.19
Component: Media Keywords: has-patch has-screenshots
Focuses: javascript Cc:
PR Number:

Description

Layman error description: When there are more than one Inline Media Uploader on a page in the Admin, and one tries to upload a new image or file to the Media Library, the upload gets stuck and the image is never inserted into the media library (see attached screenshot). This happens to me when using the post-types-definitely plugin to create multiple media fields in a CPT. The exact same error (including error messages) has been described before (https://wordpress.org/support/topic/js-bug-prevents-uploading-media-in-pagepost-editor/?replies=11#post-6860868 and https://wordpress.org/support/topic/error-uploading-media-in-post-if-yoast-is-active/#post-6922117), and even though there it was related to a plugin combination, it is not in my case. The workaround proposed there is not applicable to my case and the error source I found has nothing to do with plugins.

Technical error description: When examined in the Firefox Developer Tools, the console shows the error TypeError: this.$index is undefined. In Chrome, the error is Cannot read property 'text' of undefined. Both of them point to wp-includes/js/media-views.js:UploaderStatus.info() https://core.trac.wordpress.org/browser/trunk/src/wp-includes/js/media-views.js?rev=40359#L8361

Technical analysis: Upon closer inspection of the code, the problem seems to be as follows: When we upload a file (by clicking Ok on the OS file dialogue), a callback handler calls info() on each UploadStatus object belonging to a loaded media dialogue. The method info() relies on the fact that normally, the method ready() on the same UploadStatus object is called before the call to info(). ready() uses JQuery to set (amongst others) the member variable $index inside the UploadStatus object. info() then tries to access this.$index on all UploadStatus objects and set their contents using the JQuery text() function. However, since ready() is not called before a media dialogue is opened for the first time, and our opened dialogue calls info() on all media dialogues, the access to this.$index fails in the unopened UploadStatus objects, throwing the above Javascript Error and making the script abort, therefore freezing the upload progress bar.

Workarounds: This analysis is confirmed by the fact that when I open all media dialogues on the page at least once before uploading an image in any of them, uploading works as expected (because all of the UploadStatuses are ready() and have their $index member variable defined). A workaround for the problem is to just use the inline media dialogues for selecting existing media from the media library, and using the Media->New menu to actually upload any files. The error happened in all Wordpress versions I tested (since 4.8.3).

Proposed solution: I can see a number of different solutions for this problem. The best one would be to only call info() on the UploadStatus belonging to the opened media dialogue. However, the info() calls are performed via a single event trigger (https://core.trac.wordpress.org/browser/trunk/src/wp-includes/js/media-views.js?rev=40359#L8289) that all info() methods subscribe to in the UploadStatus.initialize() function (relevant for this bug is the "add" trigger), and this trigger cannot distinguish its subscribers. Therefore I propose to instead call ready() at the end of the initialize() function. This way, ready() is only called once more than before per media dialogue and page load, and we can be certain that info() is only called on UploadStatuses that are ready().

Attachments (4)

upload-error-chrome.png (340.1 KB) - added by carlobeltrame 23 months ago.
Screenshot of the frozen uploader including the Javascript console showing the error
wp-patch-uploadstatus.diff (481 bytes) - added by carlobeltrame 23 months ago.
Proposed patch
minimal-working-example.php (2.0 KB) - added by carlobeltrame 21 months ago.
MWE for testing the bug and the proposed fix
Screenshot from 2018-01-19 10-39-52.png (73.2 KB) - added by carlobeltrame 21 months ago.
Screenshot right before opening the media modal

Download all attachments as: .zip

Change History (10)

@carlobeltrame
23 months ago

Screenshot of the frozen uploader including the Javascript console showing the error

@carlobeltrame
23 months ago

Proposed patch

#1 @carlobeltrame
22 months ago

  • Keywords has-patch dev-feedback has-screenshots added

#2 @carlobeltrame
22 months ago

  • Keywords dev-feedback removed

#3 @carlobeltrame
22 months ago

@flixos90 since I discovered this bug while using your definitely-plugins, would you be so kind to review this bug and the fix I proposed? Thanks!

#4 @carlobeltrame
21 months ago

  • Version changed from 4.8.2 to 3.7.19

Is there anything standing in the way of merging this bug fix? Judging by the SVN blame, this bug has existed since at least version 3.7.16 in Spetember 2015.
In the meantime, people who have the same problem can fix it by adding this to their plugin or theme functions.php file:

function gloggi_fix_media_uploader() {
?><script>
    var oldUploaderStatus = wp.media.view.UploaderStatus;
    wp.media.view.UploaderStatus = oldUploaderStatus.extend({
       initialize: function() {
           oldUploaderStatus.prototype.initialize.call(this);
           this.ready();
       }
    });
</script><?php
}
function gloggi_add_media_modification_js() {
    add_action( 'admin_print_footer_scripts', 'gloggi_fix_media_uploader' );
}
add_action( 'wp_enqueue_media', 'gloggi_add_media_modification_js' );

#5 @adamsilverstein
21 months ago

@carlobeltrame Welcome to trac and thanks for the detailed bug report.

I'd like to try to reproduce the error so i can do some digging and validate the fix. Can you provide the code snippet you are using for post-types-definitely to get the multiple image uploaders in a CPT? Also, maybe a screenshot of your screen before you open the media modal so i can see that as well.

@carlobeltrame
21 months ago

MWE for testing the bug and the proposed fix

@carlobeltrame
21 months ago

Screenshot right before opening the media modal

#6 @carlobeltrame
21 months ago

Sure! I have attached the requested files. If you need anything else, feel free to ask.

Note: See TracTickets for help on using tickets.