pods-framework / pods

The Pods Framework is a Content Development Framework for WordPress - It lets you create and extend content types that can be used for any project. Add fields of various types we've built in, or add your own with custom inputs, you have total control.
https://pods.io/
GNU General Public License v2.0
1.07k stars 264 forks source link

DFV Upload prints JS error when 'Add File' button is clicked #4553

Closed jamesgol closed 6 years ago

jamesgol commented 6 years ago

With 'Upload Only (Plupload)' on a single or multi file field, clicking the 'Add File' button on a backend post generates a JavaScript error in the console

Uncaught TypeError: this.uploader.invoke is not a function
    at n.onChildviewAddFileClick (file-upload.js:80)
    at n._childViewEventHandler (view.js:203)
    at _ (backbone.min.js?ver=1.2.3:1)
    at m (backbone.min.js?ver=1.2.3:1)
    at f (backbone.min.js?ver=1.2.3:1)
    at n.u.trigger (backbone.min.js?ver=1.2.3:1)
    at n.triggerMethod$1 (trigger-method.js:40)
    at n.triggerMethod (view.js:181)
    at n.<anonymous> (triggers.js:35)
    at HTMLDivElement.dispatch (jquery.js?ver=1.12.4:3)
onChildviewAddFileClick @ file-upload.js:80
_childViewEventHandler @ view.js:203
_ @ backbone.min.js?ver=1.2.3:1
m @ backbone.min.js?ver=1.2.3:1
f @ backbone.min.js?ver=1.2.3:1
u.trigger @ backbone.min.js?ver=1.2.3:1
triggerMethod$1 @ trigger-method.js:40
triggerMethod @ view.js:181
(anonymous) @ triggers.js:35
dispatch @ jquery.js?ver=1.12.4:3
r.handle @ jquery.js?ver=1.12.4:3

Upload appears to work fine though.

    /**
     * Fired by a add:file:click trigger in any child view
     *
     * plupload fields should never generate this event as it places a shim over our button and handles the event
     * internally
     */
    onChildviewAddFileClick: function () {
        // Invoke the uploader
        this.uploader.invoke();
    },
pglewis commented 6 years ago

I was never able to reproduce this in #4418 but it was also reported there.

I've attempted to reproduce this a few different times without any luck so let me know if you can think of what might be different.

pglewis commented 6 years ago

The root of the problem is we don't need to invoke the plupload uploader so it doesn't have that method. plupload places a shim over the upload button and takes control on its own so, when working properly, the button click event never fires and the event-handler that calls invoke doesn't get called.

jamesgol commented 6 years ago

So a simple fix should be to only try to run the function if this.uploader is valid

pglewis commented 6 years ago

It sounds like it would treat the symptom but I'd really like to know why it's happening if possible.

https://github.com/pods-framework/pods/blob/b1e4098/ui/js/pods-dfv/_src/file-upload/file-upload.js?ts=4#L73-L76

Having a comment there is not an excuse to ignore checking the validity of calling invoke() but I would like to know if it's an indicator that something else isn't working right before forever suppressing our surprise via error.

pglewis commented 6 years ago

@jamesgol: can you get me a package export to test with? Sometimes these edge cases happen if there is only one field setup a certain way, sometimes it won't happen if there's just one field, sometimes it depends on the Pod type and options.

jamesgol commented 6 years ago

I could do an export but it'd be quicker just to create a new pod and add a single plupload field with defaults

pglewis commented 6 years ago

I could really use some help trying to reproduce this if anyone else has any bandwidth, @jimtrue, @nicdford, @JoryHogeveen, @sc0ttkclark

pglewis commented 6 years ago

I still get a clean console in both Chromium and FF, Jim can reproduce

nicdford commented 6 years ago

I'm able to reproduce the issue, though it reads slightly different.

Uncaught TypeError: this.uploader.invoke is not a function
    at n.onChildviewAddFileClick (file-upload.js:80)
    at n._childViewEventHandler (view.js:203)
    at _ (load-scripts.php?c=1&load[]=hoverIntent,common,admin-bar,heartbeat,autosave,suggest,wp-ajax-response,jquery-color,wp-lists,jquery-ui-core,jquery-ui-widget,j&load[]=query-ui-mouse,jquery-ui-sortable,postbox,jquery-ui-position,jquery-ui-menu,wp-a11y,jquery-ui-autocomplete,tags-suggest,tags-box&load[]=,underscore,word-count,post,editor-expand,svg-painter,wp-auth-check,shortcode,backbone,wp-util,wp-backbone,media-models,plupload&load[]=,wp-plupload,mediaelement,wp-mediaelement,media-views,media-editor,media-audiovideo,mce-view,imgareaselect,image-edit&ver=4.8.3:346)
    at m (load-scripts.php?c=1&load[]=hoverIntent,common,admin-bar,heartbeat,autosave,suggest,wp-ajax-response,jquery-color,wp-lists,jquery-ui-core,jquery-ui-widget,j&load[]=query-ui-mouse,jquery-ui-sortable,postbox,jquery-ui-position,jquery-ui-menu,wp-a11y,jquery-ui-autocomplete,tags-suggest,tags-box&load[]=,underscore,word-count,post,editor-expand,svg-painter,wp-auth-check,shortcode,backbone,wp-util,wp-backbone,media-models,plupload&load[]=,wp-plupload,mediaelement,wp-mediaelement,media-views,media-editor,media-audiovideo,mce-view,imgareaselect,image-edit&ver=4.8.3:346)
    at f (load-scripts.php?c=1&load[]=hoverIntent,common,admin-bar,heartbeat,autosave,suggest,wp-ajax-response,jquery-color,wp-lists,jquery-ui-core,jquery-ui-widget,j&load[]=query-ui-mouse,jquery-ui-sortable,postbox,jquery-ui-position,jquery-ui-menu,wp-a11y,jquery-ui-autocomplete,tags-suggest,tags-box&load[]=,underscore,word-count,post,editor-expand,svg-painter,wp-auth-check,shortcode,backbone,wp-util,wp-backbone,media-models,plupload&load[]=,wp-plupload,mediaelement,wp-mediaelement,media-views,media-editor,media-audiovideo,mce-view,imgareaselect,image-edit&ver=4.8.3:346)
    at n.u.trigger (load-scripts.php?c=1&load[]=hoverIntent,common,admin-bar,heartbeat,autosave,suggest,wp-ajax-response,jquery-color,wp-lists,jquery-ui-core,jquery-ui-widget,j&load[]=query-ui-mouse,jquery-ui-sortable,postbox,jquery-ui-position,jquery-ui-menu,wp-a11y,jquery-ui-autocomplete,tags-suggest,tags-box&load[]=,underscore,word-count,post,editor-expand,svg-painter,wp-auth-check,shortcode,backbone,wp-util,wp-backbone,media-models,plupload&load[]=,wp-plupload,mediaelement,wp-mediaelement,media-views,media-editor,media-audiovideo,mce-view,imgareaselect,image-edit&ver=4.8.3:346)
    at n.triggerMethod$1 (trigger-method.js:40)
    at n.triggerMethod (view.js:181)
    at n.<anonymous> (triggers.js:35)
    at HTMLDivElement.dispatch (load-scripts.php?c=1&load[]=jquery-core,jquery-migrate,utils&ver=4.8.3:3)
pglewis commented 6 years ago

The this.uploader.invoke is not a function part is the main thing, so that's 3/4... everyone but me.

pglewis commented 6 years ago

So a simple fix should be to only try to run the function if this.uploader is valid

We've narrowed it down to browser environment and that's not a minefield to continue through. This solution it is.