Update Drupal 6 to jQuery 1.2.3
| Project: | Drupal |
| Version: | 6.x-dev |
| Component: | javascript |
| Category: | bug report |
| Priority: | normal |
| Assigned: | Unassigned |
| Status: | closed |
A bug fix of jQuery was released today including these changes: http://docs.jquery.com/Release:jQuery_1.2.2
Larry Garfield announced this to the dev list here asking, "Were someone to write a 1.2.2 patch for Drupal 6, would it be considered?"
To which Moshe said, "Yes ... Further I think that a pure bug fix release of Jquery is acceptable even after 6 goes out. We wouldn't rush out a release just for that, but i expect such a patch would be accepted."
And Gábor said, "IMHO we should consider on the grounds of Drupal functionality. Such a patch would need all JS functionality tested in "all" browsers (== the browsers people tested previous JS functionality in)."
Well, here's a patch to kick things off.
| Attachment | Size |
|---|---|
| jQuery122_update.patch | 55.5 KB |

#1
In order to test this, please post the results of all of the tests listed in the JAVASCRIPT section at http://groups.drupal.org/node/5974, for each browser(s) you test them in.
#2
Block, book, menu, input format, and profile drag and drop plus the poll "add more" button, should probably be added to that list.
#3
this breaks tableheader and collapsible fields on Mac OS Safari 2, 3, FF 2 and Opera 9 - however drag and drop works.
#4
I've posted suggested updates to webchicks list here: http://groups.drupal.org/node/5974#comment-24886
#5
jQuery's 1.2.2 release announcement says:
which is surely what's broken tableheaders.
http://dev.jquery.com/changeset/4089/trunk/jquery
http://dev.jquery.com/changeset/4251/trunk/jquery
at least two of the changes.
#6
gah, will look into table headers.
#7
Just did a quick test and the problem is "context". The changes catch mentioned could be causing other problems but it's not running at all.
On line 12:
$('table.sticky-enabled thead:not(.tableHeader-processed)', context).each(function () {Remove context and it runs fine for me in FF & Safari. This also applies to collapse.js.
[edit: It looks like this affects *a lot* of scripts.]
#8
Looks like it is not just a bug fix release after all? Or we were depending on bugs? :)
#9
The jQuery blog calls it "primarily a bug fix and optimization release", but goes on to list several entirely new features.
"New Events API", ".ready() overhaul", ".bind(”mousewheel”)" and ":not()" go beyond the scope of bugs, so I guess we shouldn't be surprised by compatibility troubles.
#10
Ughm, not nice (TM).
#11
We need someone more familiar with jQuery to look into this. With some brief testing, it looks like most things work with the context object removed from the selection but I'm not sure why it was set-up like that to begin with.
Is it to narrow the selected scope for performance reasons?
Why?:
Drupal.behaviors.collapse = function (context) {$('fieldset.collapsible > legend:not(.collapse-processed)', context).each(function() {
...
#12
dvessel, this may help...maybe..
An explanation
http://drupal.org/node/114774#javascript-behaviors
When it got added
http://cvs.drupal.org/viewvc.py/drupal/drupal/misc/collapse.js?r1=1.13&r...
Nedjo and recidive worked on it back in Feb - Jul last year.
http://drupal.org/node/120360
#13
Ah, thanks momendo. So, it's primarily for ahah.js so it doesn't loose its state when loading new content. In *all* other cases "context" points to the whole document. Something in context is not being passed along.
I'll try to look into it further but please, if anyone here is familiar with this.. Jump right in.
#14
Do you have an estimate of how many already released contribs will get broken if we push this into Drupal 6? This does not look like a happy situation at all :|
#15
Well it's much easier for contribs to update for jQuery compatibility (and new features) during the release cycle than core, and they're very likely to. This isn't an estimate, but I can only see a handful of modules in the project/name 6.x filtered list that do much with jQuery, so it's likely to be a small subset of even those.
#16
Gábor, it's hard to say at this point. If this was the only problem, this patch should not affect contrib.
With the .ready() overhaul, it doesn't properly pass the document object when structured as it is now.
$(document).ready(Drupal.attachBehaviors);It's been changed to this and it works for me:
$(document).ready(function() {Drupal.attachBehaviors(this);
});
Anyone see a problem with this? At least we can start testing.
#17
Good catch dvessel. I did some testing..
IE7, Safari 3.0.4 mac/vista, FF2, Opera 9.5 beta
Autocompletion in user field fails, ALL browsers
/node/add/page
Error: invalid label
Source File: http://dev/drupal/user/autocomplete/m
Line: 1, Column: 11
Source Code:
{ "momendo": "momendo" }
Error: matches has no properties
Source File: http://dev/drupal/misc/autocomplete.js?m
Line: 275
if (typeof matches['status'] == 'undefined' || matches['status'] != 0) {
In IE7
Tableheader tracking fails
admin/user/rules
admin/build/modules
#18
Yeah, this is beyond me. Will need some serious help.
I also noticed autocomplete. Broke in all browsers pft.!
.clone() (used in tableheaders.js) has been reworked breaking IE7 and if there's a checkbox in it, an exception is thrown.
IE6 seems fine outside of autocomplete.
Most other areas in FF2 and Safari 3 looks fine. Drag & Drop table rows, Book outlines, teaser splitting, etc..
Opera 9.25 feels slow and flakey with collapsible fieldsets and floating table headers..
Safari 2 teaser splitting is broke as is configuring book outlines.. "node/#/outline"
This doesn't look good for a point release. We might just want to skip this release.
#19
No worries. I tracked down the autocomplete bug.
There seems to be some discussion about how jQuery changed how relative and absolute paths are used when doing JSON calls with the GET method.
http://dev.jquery.com/ticket/2174
The code in question...
http://dev.jquery.com/changeset/4123/trunk/jquery/src/ajax.js
Proposed fix.
http://dev.jquery.com/attachment/ticket/2174/2174.diff
http://groups.google.com/group/jquery-dev/browse_thread/thread/62a4f75f6...
The fix is to switch to a POST method.
Please check.
#20
It seems IE7 doesn't like clone(true) in tableheader.js
http://cvs.drupal.org/viewvc.py/drupal/drupal/misc/tableheader.js?view=m...
var headerClone = $(this).clone(true)Some background that may help...
http://dev.jquery.com/changeset/4089/trunk/jquery
http://dev.jquery.com/ticket/1836
#21
Book outline works for me in IE7, Opera 9.5, FF2 and Safari 3.0.4
node/#/outline
#22
Upgrading to such a backward compatibility breaking point release should not be on our critical path IMHO. If all the above "bugs" are fixed in our code, then this might still get into Drupal 6, but I would not call this critical.
#23
Guys, what sort of timeline are you on, here? We've already tackled the JSON issue and are looking at the TR cloning issue. We could, possibly, get a 1.2.3 out here soon if it would make a difference. Let us know.
#24
John: good to get some feedback from you as well. Our ideal timeline calls for a Drupal 6 release in around 2 weeks. That requires all critical issues resolved as well. But to have a jQuery update in core, we would need critical testing, which would need to have fixes out in no more then a week or so.
#25
I've been working with getting our scripts updated for jQuery 1.2.2, but there are some perplexing issues (the return data from JSON requests was most puzzling). Syntactically, it looks like our scripts are written correctly. While possible to write things so they'll work, these changes really shouldn't be necessary. Waiting for 1.2.3 sounds like very good option.
#26
Waiting for jQuery 1.2.3 sounds good but that may not happen before Drupal 6 is ready. Or am I wrong John?
Gabor mentions Drupal 6 may be ready in about 2 weeks, that's the end of January. Do you think you could have jQuery 1.2.3 ready before that John?
What Quicksketch mentions about JSON return data worries me a little, many things depend on JSON.
jQuery 1.2.2 says it has ' 300% Speed Improvements to $(DOMElement)', that will really help many sites. I've worked on a few Drupal projects that rely so much on jQuery that speed improvement could help a lot of pages to run better.
#27
I sent John Resig an e-mail asking for an update on the status of a potential jQuery 1.2.3 release.
#28
We may be able to move to get something ready by this weekend 26/27th. For right now, could you guys test the current jQuery nightly?
http://code.jquery.com/jquery-nightly.js
http://code.jquery.com/jquery-nightly.pack.js
Most of our fixes have already landed and if these help you then that'll make things easier for us, in testing.
#29
Here's a patch w/ the current nightly (rev 4513). Not tested in the least, but this should make it easier for folks to do so.
#30
Thank you John! I'll be testing tonight.
#31
We need to add back dvessel's #16 bind fix.
Preliminary testing...
-The autocomplete bug is fixed.
-The IE7 tableheader tracking bug is fixed... mostly.. There's a blue 2px border line from the header lingering way above at the top of the page. I think it isn't being hidden correctly.
#32
@momendo: I've spotted some wierdness with IE7 tables/borders recently but hadn't filed an issue yet, are you sure that's because of this patch?
#33
great! #31 works on Mac OS FF 2.0.0.11, Safari 3 (super fast!), safari 2, and Opera 9.24 (slow).
I tested tableheader, drag n drop (menus, blocks), collapsible fields and autocomplete with no problem.
#34
The IE problem is pointed out here: http://drupal.org/node/213375
#35
While not a bug report per se, I'm changing the status and bumping to critical. We need this sorted prior to the final release of D6.
#36
node/add/story
Check if collapsing/expanding fieldsets work.
IE6 ok, IE7 ok, Saf2 ok, Saf3 ok, FF1.5 ok, FF2 ok, Op9.25 ok
note: Opera on the Mac struggles really bad. Feels very slow.
node/add/story
Check if the autocomplete form fields (e.g. "Authored by") work.
IE6 ok, IE7 ok, Saf2 ok, Saf3 ok, FF1.5 ok, FF2 ok, Op9.25 ok
node/add/story
Check if the teaser splitter works.
IE6 ok, IE7 ok, Saf2 fail, Saf3 ok, FF1.5 ok, FF2 ok, Op9.25 ok
node/#/outline
With book.module enabled: Check if the parent selector works.
IE6 ok, IE7 ok, Saf2 ok, Saf3 ok, FF1.5 ok, FF2 ok, Op9.25 ok
note: Safari 2 jumps to the top when altering the book outline from 'node/add/story'.
node/add/story
With upload.module enabled: Test attaching a new file.
IE6 ok, IE7 ok, Saf2 ok, Saf3 ok, FF1.5 ok, FF2 ok, Op9.25 ok
any Check if logging in with OpenID works.
Untested.
admin/content/node
Check if a row gets highlighted when selected.
IE6 ok, IE7 ok, Saf2 ok, Saf3 ok, FF1.5 ok, FF2 ok, Op9.25 ok
Check if you can select multiple rows with shift+click.
IE6 ok, IE7 ok, Saf2 ok, Saf3 ok, FF1.5 ok, FF2 ok, Op9.25 ok
Check if you can select all rows by clicking at the checkbox in the table header.
IE6 ok, IE7 ok, Saf2 ok, Saf3 ok, FF1.5 ok, FF2 ok, Op9.25 ok
note: small issue with checked state is not mirrored from floater to parent table. Not related to this patch.
admin/user/access
Scroll down the page. The table header should keep floating at the top of the viewport.
IE6 disabled, IE7 ok, Saf2 ok, Saf3 ok, FF1.5 ok, FF2 ok, Op9.25 ok
note: In Opera, cell widths are further misaligned. It wasn't perfect before but it's worse now. Resizing the window causes intermitent pauses. It works but it's flakey.
admin/build/themes/settings/garland
Check if the color picker works.
IE6 ok, IE7 ok, Saf2 ok, Saf3 ok, FF1.5 ok, FF2 ok, Op9.25 ok
admin/user/user/create
Check if the password strength indicator and password match indicator works.
IE6 ok, IE7 ok, Saf2 ok, Saf3 ok, FF1.5 ok, FF2 ok, Op9.25 ok
admin/settings/clean-urls
See if it detects clean urls.
IE6 ok, IE7 ok, Saf2 ok, Saf3 ok, FF1.5 ok, FF2 ok, Op9.25 ok
admin/build/menu-customize/navigation
Drag and drop menu settings.
IE6 ok, IE7 ok, Saf2 ok, Saf3 ok, FF1.5 ok, FF2 ok, Op9.25 ok
admin/build/block
Move blocks by drag and drop. Check with drop down menu too.
IE6 ok, IE7 ok, Saf2 ok, Saf3 ok, FF1.5 ok, FF2 ok, Op9.25 ok
admin/content/taxonomy/#
Drag & drop hierarchy of terms within a vocabulary.
IE6 ok, IE7 ok, Saf2 ok, Saf3 ok, FF1.5 ok, FF2, Op9.25 ok
That was the most mind numbing task. Overall, it's solid. Only Safari 2 and Opera has some issues.
#37
subscribe - nice test suite, dvessel.
#38
I've tested IE7, Saf3, FF 2, Op9 (Win) and can confirm dvessel's findings above.
Do we need to carry out a similar test on other OS?
#39
Anyone is working on solving those? If not, then it is unlikely that we can get this to Drupal 6.0 final!
#40
Followup from dvessel's testing:
node/add/story
Check if collapsing/expanding fieldsets work.
- Can confirm the patch makes this slow in Opera on XP.
admin/user/access - well I don't have enough rules set up to test properly, but permissions looked pretty clean in Opera to me.
So it seems to be 1. slowness on fieldsets in opera 2. teaser splitter and book outline in Safari 2
Don't have another platform (or Safari 2) to test on here.
#41
node/add/story
on Opera on Mac OS, the collapsed fields are painfully slow to get expanded, it takes 2 sec to expand. may not sound long but it is!
further testing on Safari 2.0.4 Mac OS: confirmed the teaser splitter is broken (see screenshot); also, the menu drag and drop works but seem to have some CSS issues (see screenshot).
#42
Can we verify that those Safari 2 issues were present in jQuery 1.2.1? If so, then those are unreported bugs. In Safari 3.0.4 and Opera 9.5 on Mac and Windows, I don't see any issues from #40 & #41.
#43
momendo is right, my Opera was out of date.
I upgraded to 9.25 and it was a little faster (but choppy) - much less than 0.5 seconds even on large fieldsets like the content types form. I also tried 9.5b and it was completely smooth and very fast. So I think we can assume this is an Opera bug that's been fixed in the latest release. I don't think that's a showstopper, at all.
So just Safari 2 left now. Note the teaser splitter is disabled completely in Konqueror, and was quite broken in Opera for some time before a recent fix got in. If it's an old unreported bug, then there's possibly no new bugs introduced here at all, so marking to needs review at least until that's confirmed either way.
#44
@momendo: safari 3 is doing well so far.
I tried the latest Opera 2.50 beta on Mac OS, and it seems to generally cope better. collapsible fields are fast now. The only issue I found was when resizing the window, the tableheaders stop working. So I think overall Opera is not a big issue.
#45
Updating to latest r4537 build from r4513. It may fix some Safari issues. Not sure if there are any significant improvements.
Diff from jQuery
http://dev.jquery.com/changeset?old_path=%2Ftrunk%2Fjquery&old=4513&new_...
#46
@scor: how about safari 2 unpatched vs. patched on those two things that dvessel flagged?
Opera 9.25 I don't get disappearing tableheaders on resizing patched or unpatched.
Opera 9.5b I do get them, patched or unpatched.
So this is a new bug introduced by 9.5 which is independent of this patch being applied or not. Could possibly be a new issue, but again we're back to Safari 2.
Marking back to needs review again for the same reasons as #43.
#47
momendo, Safari 2 didn't break in 1.2.1. If you or anyone wants to test, you can install multi-safari. Make sure you install the alternate version for 10.5.1.
The problem comes from .slideDown('fast') within "split_teaser()". I replaced it with .show() and it worked.
Just to reiterate, fixes between 1.2.2 & 1.2.3:
Persisting problems:
#48
Whoops, crosspost
#49
Does that mean it's only .slideDown() in Safari 2 then notwithstanding the Opera fieldset bug which seems version specific?
jQuery bug report for .slideDown() anyway: http://dev.jquery.com/ticket/2246
#50
We don't only need all our issues fixed, but a jQuery 1.2.3 released as well to ship with. John, what is the likeliness of this in the coming days?
#51
Thanks catch for filing the report. And yes, it only affects Safari 2. It causes it to move out of the document flow.
The issue with Opera seems to stem from *setting* dimensions under some odd condition. tableheader and the field set may be related.
#52
folks are reporting that 1.2.3 is passing all the test suite. not sure if that means john will release this week or later.
#53
Safari 2 is fixed with the latest nightly.!
The only problem left is Opera. It's happening because of how collapse.js tries to "scroll" into view. This has caused another problem with Epiphany and I think the animation should be removed so it behaves more like Drupal 5. Anyone else agree?
tableheader.js I'm just not sure about. I tried my best with it but it was never running perfectly with Opera.
#54
@dvessel: if you mean when big fieldsets do that slidedown thing on the whole page, I remember a patch that broke it/took it out, and I preferred it when they just snap into view personally - seems faster, and having your page flash past your eyes isn't that exciting imo.
#55
Yeah catch, exactly. I'll roll a patch in that queue and we'll see what others think of it.
#56
OK I re-read and it's the exact same issue, now I remember!
That stuff happens really, really quick, and I never knew it was there, nor did I notice when it wasn't there until someone pointed it out.
Couple options then: disable the 'scroll into view' in opera, leave it in since it's fixed in 9.5b anyway, or remove it completely. Looking forward to the patch :)
#57
subscribing
#58
I was mistaken about the scrolling behavior so I decided to leave it alone. :/
But I did find the line causing the huge slowdown in Opera9.25 Mac/Win. It's due to .offset() being used in both tableheader.js and collapse.js.
Inside the latest nightly on line 3337 –the test for jQuery.css(parent, "display"):
jQuery.fn.offset = function()
...
// Get parent scroll offsets
while ( parent && parent.tagName && !/^body|html$/i.test(parent.tagName) ) {
// Remove parent scroll UNLESS that parent is inline or a table to work around Opera inline/table scrollLeft/Top bug
if ( !/^inline|table.*$/i.test(jQuery.css(parent, "display")) ) // <<<< LINE 3337 !!
// Subtract parent scroll offsets
add( -parent.scrollLeft, -parent.scrollTop );
// Mozilla does not add the border for a parent that has overflow != visible
if ( mozilla && jQuery.css(parent, "overflow") != "visible" )
border( parent );
// Get next parent
parent = parent.parentNode;
}
I don't know how to fix that and the issue tracker used for jQuery keeps giving errors.
edit: dug a little deeper and it's due to this fix. http://dev.jquery.com/ticket/2037
#59
We're going to look into the Opera performance hit and we'll shoot for a final release of 1.2.3 this weekend.
#60
Considering Opera 9.25 has low browser usage (less than 2% overall) and is not an issue in 9.5 beta which will be released in a few weeks, I'd say the Opera 9.25 slowness is not a showstopper, if we are held up at some point because of that issue.
#61
@momendo, try this: chx uses opera.
#62
It would be nice for 1) us to be using an actual release instead of a nightly (I'd say John is in agreement about this... http://ejohn.org/blog/open-source-release-syncing/ ) 2) this Opera performance bug to be fixed.
But, most Opera users upgrade immediately upon a release (including chx) so I don't think that the Opera performance bug should hold back either a new jQuery release or a Drupal release.
#63
We should have another slot for core maintainer focused on jQuery to better communicate with Johns' team. Any takers? Post on the dev mailing list.
The Opera problem I'm not sure about. The jQuery ticket that caused this change wasn't for a real world problem. It was due to a failure in a test suite.
#64
Kestrel (AKA Opera 9.5) *won't* be out in a few weeks (I'm sorry I can't be more specific), so please keep that in mind when deciding how to handle the performance issue. I've asked some of my co-workers to take a look, but I'm not sure they'll be able to prior to the release of jQuery 1.2.3 or if they have the experience with jQuery to do the necessary debugging to find a better work-around.
PS: I work for Opera Software, in case that wasn't clear from the above.
#65
I've posted an alternative fix for http://dev.jquery.com/ticket/2037 that solves the performance problems.
#66
Thank you Junyor. I can confirm that it solves the slowdown.
#67
Hey Guys - We pushed 1.2.3 live tonight, with the above fix included - let's hope for the best!
http://code.jquery.com/jquery-1.2.3.js
#68
Thank you John. This should be good to go.
Who wants to do a final review?
#69
Tableheaders and fieldsets working great on FF2, IE7, Safari 3, Opera 9.25 and Opera 9.5/Kestrel (and fieldsets fine in IE6). Sticky tableheader in Kestrel still disappears on resize - this is the same patched as unpatched, as before, so not an issue but worth noting.
I don't see any reason why this shouldn't get in now, but leaving it for one more review.
#70
Tested with FF3 as well. Tableheaders, fieldsets, textareas and the teasersplitter are all working very fine. Do we want another all-browser review, or is this ready?
#71
Oh and the teaser splitter is fine on Opera and Safari 3(XP) as well.
#72
I'm testing dvessel's suite on mac browsers at the moment. will report once done.
#73
Okay, I tested this on Mac OS Tiger 10.4 with FF, Safari and Opera..
Seems things are working pretty smooth and without errors..
To me - and according the above reviews - this is indeed ready..
Goba, fire up your patch applier and get this sucker in! :-)
#74
Thanks John, dvessel, catch and all others testing the patch. Committed to 6.x. Still needs to be committed to 7.x
#75
on mac OS, no major problem to report in FF2, FF3beta2, safari 3.0.4, opera 0.24 and 0.50b.
In opera 9.50beta, the tableheader is choppy, and sometimes doesn't work.
In safari 2.04 I found the teaser splitter is still not usable as I mentioned in #41.
However, safari 3 is now available, and safari 2 should not block drupal 6, so that's definitely RTBC.
#76
cross posting
#77
Also note that COPYRIGHT.txt needs to be updated to 2008.
#78
Cross posted. But we do need a 2007->2008 patch.
#79
@keith.smith: good call - http://drupal.org/node/218436
#80
A tip for anyone else who is feeling groggy this morning: if you are not seeing this update, remember to move from HEAD to the DRUPAL-6 branch (/me slaps head).
#81
Committed to CVS HEAD. Moving back to D6.
#82
#83
Automatically closed -- issue fixed for two weeks with no activity.
#84
I'd like to invite you all to #256285: Upgrade to jQuery 1.2.6