Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
Probably not nearly as important as #944308: Update to jQuery 1.4.4, so it could likely be tackled in a point release, but it does have a few bug fixes and enhancements for jQuery 1.4 (https://github.com/malsup/form/commits/master).
Comment | File | Size | Author |
---|---|---|---|
#19 | jform.patch | 19.05 KB | droplet |
#17 | drupal.jquery_form_252.patch | 20.72 KB | threewestwinds |
#16 | drupal.jquery_form_252.patch | 20.72 KB | effulgentsia |
#4 | 969456_jquery-form-2.49.patch | 20.26 KB | amateescu |
#1 | 969456_jquery-form-2.49.patch | 19.81 KB | amateescu |
Comments
Comment #1
amateescu CreditAttribution: amateescu commentedNot bad.. here's a patch for testing.
I used Closure Compiler to minify the source and the filesize is smaller by a few kb.
Comment #2
cosmicdreams CreditAttribution: cosmicdreams commentedCan someone compile a conclusive list on what needs to be tested in order to prove that this update doesn't break anything? I know the testing architecture does a pretty good job of this already. But I'm willing to put in the time necessary in order to prove on a practical level that this update is ready to commit.
Comment #3
andypostShould be tested under macos and linux.
Windows FF3, opera 10, IE 9 - works
Comment #4
amateescu CreditAttribution: amateescu commentedForgot to change the version in system.module.
Comment #5
andypostWe need more testing! Can someone explain places where this lib is used.
Comment #6
cosmicdreams CreditAttribution: cosmicdreams commentedI'm happy to help here, I suppose I could go to every page where the form javascript is loaded an test every behavior on the screen. But, it seems that the library's purpose to allow ajax form submissions. So, it would seem that we should test every form and every form submisssion.
That may take some time. But the number of tests necessary are finite.
Comment #7
mfer CreditAttribution: mfer commentedLooking thought the commit logs and details there are a number of things we should check. Since v2.38 updated the success callback for jQuery 1.4+ and some other 1.4 changes were made I'm tempted to call this critical. Things we should, at least, test:
Comment #8
andypostfirst and third I've tested already under win 7/2003 except IE6-7
Comment #9
cosmicdreams CreditAttribution: cosmicdreams commentedAh very good, I'll be able to test this tonight. This may be able to help #951500: HTTP error 0 on ajax calls in Chrome
Comment #10
cosmicdreams CreditAttribution: cosmicdreams commentedI tested with the following so far tonight:
Chrome 9 (Canary Build), Chrome 7, FF 3.6, IE 9 beta
Haven't run into any issues with file uploads, regular submission of forms, or any other ajax related behaviors.
I intend to test IE 8, and IE 7 still tonight. I'll update this post when I've completed that work.
Comment #11
cosmicdreams CreditAttribution: cosmicdreams commentedone thing I'm noticing is that it is taking an awefully long time for the overlay to complete a simple ajax call. Such a navigating from People to Modules. I don't know if that is because of this patch or not.
Comment #12
cosmicdreams CreditAttribution: cosmicdreams commentedSo if no one can confirm performance degradation this patch is RBTC.
Comment #13
webchickThe modules page is indeed pretty slow to load, but that doesn't seem to be caused by this patch; it's true in HEAD as well. It'd be interesting to see if reverting the jQuery 1.4.4 upgrade fixes this though. Hm...
Anyway, committed to HEAD with a CHANGELOG.txt adjustment. Thanks!
Comment #15
effulgentsia CreditAttribution: effulgentsia commentedFYI: This caused a regression described in #999704: Image field add one more button doesn't work in IE789. Patch to fix it is in #995854-70: #ajax doesn't work at all if a file element (or enctype => "multipart/form-data") is included in the form. I'm about to post a summary in the latter issue with more detail.
Comment #16
effulgentsia CreditAttribution: effulgentsia commentedMight as well get current before the 7.0 release. Here's 2.52, which reflects 3 commits made in the first week of December: https://github.com/malsup/form/commits/master/
As a separate issue, would be good to get some reviews of #1009862: Update to jQuery UI 1.8.7.
Comment #17
threewestwinds CreditAttribution: threewestwinds commentedPatch needs a reroll to commit cleanly. Since it's just a reroll and the only change was in CHANGELOG.txt, I should still be safe in still reviewing.
In Firefox 3.6 and Chrome 8, I tested the node creation form with single and multivalued fields image fields, including successful submission of the form. I also used Field UI successfully to create said fields. Since jquery.form is attached to any ajax element, I also ran through the AJAX Examples module quickly (in FF only). Everything was fully functional.
Given that #995854: #ajax doesn't work at all if a file element (or enctype => "multipart/form-data") is included in the form is fixed (which involved testing in all major browsers), we should be good to go. As per IRC with webchick, we're good to go.
Comment #18
webchickGreat. Thanks a lot for doing that testing.
Committed to HEAD. Thanks!
Comment #19
droplet CreditAttribution: droplet commentedNew patch broken all AJAX-like upload in IE
#1018714: Image Upload Widget Not Working in IE8
I have limited JS knowledge..I cannot explain so much but I catch at the line of .....
seems like "textContent" is not supported in IE. replace it with "innerHTML", it works again.
attached a patch for testing
ref: http://www.quirksmode.org/dom/w3c_html.html
Comment #20
threewestwinds CreditAttribution: threewestwinds commentedI asked webchick in IRC if we needed IE testing before this was committed. I guess not having it bit us in the butt here - it was all very last minute to get this in before 7.0 released.
See https://github.com/malsup/form/issuesearch?state=open&q=textContent#issu... for a jquery.form issue pointing out the same thing.
What are Drupal's policies with modifying external libraries like this? I'd imagine we're hesitant to take on the additional burden of support that our own custom versions would entail. It might be best to wait for jquery.form to come out with a new version that fixes this and upgrade.
Comment #21
effulgentsia CreditAttribution: effulgentsia commentedIf jquery.form.js has code that fails in IE, that is unfortunate. Good thing it's an issue in malsup's queue. Thanks for hunting it down!
For our purposes, we can work around it, as per #1018714-4: Image Upload Widget Not Working in IE8. Returning the JSON response in a TEXTAREA when delivering to an IFRAME is the recommended approach anyway.
Therefore, marking this issue back to fixed, as per #18.