Problem/Motivation

During the AJAX request a form may be rebuilt.

If an element has attached Drupal.settings, it may be added twice (for the same identifier).

Thus, when file_ajax_upload() merges these settings, it does so using array_merge_recursive() and turns scalar values into arrays of scalar values.

This results in the JS engine halting with the error:
TypeError: element_settings.url.replace is not a function

Proposed resolution

Use drupal_array_merge_deep() instead.

Remaining tasks

- Create a patch

User interface changes

None

API changes

None

Data model changes

None

CommentFileSizeAuthor
#6 file_ajax_upload-2870289-6.patch604 bytesDavid_Rothstein
#2 file_ajax_upload-2870289-2.patch974 bytesmarkhalliwell

Comments

markcarver created an issue. See original summary.

markhalliwell’s picture

markhalliwell
Primary language English
commented
Issue summary: View changes
Status: Active » Needs review
StatusFileSize
new file_ajax_upload-2870289-2.patch974 bytes
hass’s picture

hass commented
Status: Needs review » Reviewed & tested by the community

I can confim the patch solve the issue in file module.

Aside, I also get this views carousel (views_bootstrap module) issue:

Uncaught TypeError: Cannot read property 'carousel' of undefined
    at HTMLDocument.<anonymous> (views-bootstrap-carousel.js?ook5ex:5)
    at c (jquery.min.js?v=1.10.2:4)
    at Object.add [as done] (jquery.min.js?v=1.10.2:4)
    at init.ready (jquery.min.js?v=1.10.2:4)
    at $.fn.init.init (jquery.min.js?v=1.10.2:4)
    at $.fn.init (drupal.js?ook5ex:26)
    at x (jquery.min.js?v=1.10.2:4)
    at Object.attach (views-bootstrap-carousel.js?ook5ex:4)
    at Object.<anonymous> (drupal.js?ook5ex:76)
    at Function.each (jquery.min.js?v=1.10.2:4)
(anonymous) @ views-bootstrap-carousel.js?ook5ex:5
oriol_e9g’s picture

oriol_e9g
Primary language Catalan
Location Barcelona
commented

Yes, same problem and this fixs works in my case. RTBC+

stefan.r’s picture

stefan.r commented
Issue tags: +Pending Drupal 7 commit, +Drupal bugfix target
David_Rothstein’s picture

David_Rothstein commented
StatusFileSize
new file_ajax_upload-2870289-6.patch604 bytes

Looks good - interesting that no one noticed this before since we fixed the other settings-merging code to do this years ago!

I think it would be nicer to use drupal_array_merge_deep_array() here, though, since that's what all the other settings-merging code does and because it looks cleaner. Here's a patch that does this. I'm leaving it RTBC since I'm positive it's the exact same thing, but it would be good for someone else to do a quick review before committing it.

David_Rothstein’s picture

David_Rothstein commented

Removing issue credit from myself, since my contribution to this was extremely minimal.

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 6: file_ajax_upload-2870289-6.patch, failed testing.
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

David_Rothstein’s picture

David_Rothstein commented
Status: Needs work » Reviewed & tested by the community

  • stefan.r committed 7edef74 on 7.x 
    Issue #2870289 by markcarver: file_ajax_upload() causes malformed Drupal...
stefan.r’s picture

stefan.r commented
Status: Reviewed & tested by the community » Fixed
Issue tags: -Pending Drupal 7 commit

Committed and pushed to 7.x, thanks!

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.