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.
Although jQuery 1.3 final isn't out yet, it is in beta testing. John Resig has asked us to Help Test jQuery 1.3 Beta 1 and I can't think of a better way of helping them out and testing jQuery 1.3 other then sticking it straight into Drupal core. Drupal 7 will come out long after jQuery 1.3 will be out, so we'll have to eventually do this sometime anyway....
Comment | File | Size | Author |
---|---|---|---|
#77 | ff-garland-block-sort-hack.patch | 1.07 KB | mfer |
#75 | ff-garland-block-sort-hack.patch | 569 bytes | mfer |
#73 | ff-garland-block-sort-hack.patch | 611 bytes | mfer |
#71 | ff-garland-block-sort-hack.patch | 641 bytes | mfer |
#65 | screenshot2.png | 25.24 KB | RobLoach |
Comments
Comment #1
RobLoachComment #2
RobLoachComment #3
drewish CreditAttribution: drewish commentedsubscribing.
Comment #4
David StraussSubscribing.
Comment #5
catchWe have two choices:
1. Commit this without manual testing and fix/report bugs as we find them.
2. Do at least cursory testing in all major browsers for uploading, teaser splitter, sticky tableheaders, drag and drop, color.module, fieldsets (and etc.) before we commit.
Which option is best probably depends on whether an real humans visit Drupal 7 pages other than admin/build/testing - not sure what the answer is to that.
Comment #6
David Strauss@catch I vote for option two. God knows none of the devs use IE on an extremely regular basis for D7 dev.
Comment #7
catchIn that case I refer the gallery to http://groups.drupal.org/node/5974
However, I'm not at all interested in doing this - especially since we'll have to do it all over again for each jQuery beta then RC for the next few weeks/months until a full release - and also because there's pretty much zero chance that we'll release Drupal 7 with an extremely outdated jQuery version.
So if no-one steps up, rather than holding our breaths I think we should go for option #1 - I know a couple of people are running personal/testing sites on D7 and keeping contrib modules up to date on it, so we should at least be able to cover ff/opera for that.
Comment #8
David Strauss@catch That makes sense, and I certainly won't block efforts to include jQuery 1.3 betas in HEAD now. I was simply arguing for a one-time "check if jQuery 1.3 Beta 1 breaks anything obvious" check. After our initial inclusion of jQuery, we shouldn't see enormous numbers of changes.
Comment #9
Damien Tournoud CreditAttribution: Damien Tournoud commentedAs far as I know, we try to *never* commit untested code. So I vote for option #2.
Comment #10
mfer CreditAttribution: mfer commentedDue to the time it takes to test a jQuery to the point where it will be committed I would suggest waiting until jQuery 1.3 is actually out. I did it for jQuery 1.2.6 so I know it takes time.
If someone wants to do it to provide feedback for the jQuery team go for it.
Comment #11
RobLoachjQuery 1.3 Beta 2 is out.
Comment #12
RobLoachComment #13
sunI'm rather opposed to this. Our friends at jQuery have proven many times that they know how to break functionality in minor versions, i.e. point releases. IMHO, it adds enough burden to us when we have to deal with those issues in a 1.3.1 to 1.3.2 maintenance releases. Bear in mind that our Drupal jQuery gurus can work on more interesting stuff than fixing weird compatibility issues at this point in time.
Comment #14
RobLoachHere's an intermediary patch for 1.3b2. Seems to be working in the most common places.
Comment #15
stella CreditAttribution: stella commentedjquery 1.3 was released today FYI
Comment #16
catchComment #17
RobLoachThe release notes features some upgrade notes.
One thing of note is in tableheader.js, where we check if we're running < IE7:
attach: function(context) {
// This breaks in anything less than IE 7. Prevent it from running.
if (jQuery.browser.msie && parseInt(jQuery.browser.version, 10) < 7) {
return;
}
I'm not running a platform that has IE, so would someone mind seeing if this works in IE6 now? The least we should do is switch to jQuery.support for browser specifics.
There was some interesting discussion on IRC of switching from Drupal.behaviors to custom jQuery events and live() and die() to attach and detach the behaviors. If we do decide to do that, it will be handled in a separate issue.
Comment #18
mfer CreditAttribution: mfer commentedFrom the list at http://groups.drupal.org/node/5974#javascript what still needs to be tested?
Comment #19
webchickSubscribing. :)
Comment #20
starbow CreditAttribution: starbow commentedsubscribing
@Rob Loach - I got really excited when I first saw .live, but I don't think it is powerful enough to replace Drupal.behavior. It can only attach to a handful of event types, so, for example, there would be no way to attach to the scroll event in the tableheaders behavior. Maybe I am missing something though.
Comment #21
Dave ReidSee also #335931: IE7 sticky table header does not work for the IE tableheader problem.
Comment #23
mfer CreditAttribution: mfer commented@starbow you can create your own event types and they work with .bind and .live. But, that is for another time. Once we get jQuery 1.3 in core, perhaps.
Comment #24
cburschkaSubscribing...
Comment #25
korvus CreditAttribution: korvus commentedsubscribing
Comment #26
starbow CreditAttribution: starbow commentedJust to keep the ball in play, here is the patch with the tiny change needed to get it to apply against head as of this moment.
I did some light testing in FF3, taking a look at the Article create page (autocomplete, teaser splitting, text area resizing, and collapsible) and the Navigation Menu page (tableheaders and table drag+drop). The Navigation menu page works fine, but the Article create page is kicking up on of those "Warning: Unresponsive Script" boxes. It seems to be stuck in line 19, which is the big old Sizzle min line. IE7 generates a similar "Stop running slow script" box.
Comment #27
mfer CreditAttribution: mfer commentedOn the node/add/[type] pages the error seems to be coming from textarea.js and the line that says:
In particular it gets stuck further down in the is method.
Comment #28
mfer CreditAttribution: mfer commentedThe problem noted in #27 is a jQuery bug that will be fixed when 1.3.1 is released. This is expected in the next week or two. The issue details are at http://dev.jquery.com/ticket/3837.
Comment #29
RobLoachhttp://blog.jquery.com/2009/01/21/jquery-131-released/
Comment #31
starbow CreditAttribution: starbow commented1.3.1 has fixed whatever what triggering the infinite loop on the Article creation page.
What is the state of the art for testing behaviors?
Comment #33
starbow CreditAttribution: starbow commentedOk, this is my first patch to go into automated testing. Can someone help me interpret "Failed: Invalid PHP syntax." (http://testing.drupal.org/pifr/file/1/350275_4.patch)? Is my patch in a bad format (it's what eclipse kicks out), or is there actually something wrong with the code, and if so, where?
Comment #34
David StraussChecking the patch now...
Comment #35
David StraussThe patch applied fine for me, and I didn't get syntax errors on PHP 5.2.8. Here's a reroll-ish thing for good measure.
Comment #36
David StraussComment #37
RobLoachThis patch fixes a small bug with the teaser.context, switching to teaser[0] fixed it. I also ran through the tests provided in the Drupal Core Test Suite, here are the results:
Check if you can select multiple rows with shift+click.
Check if you can select all rows by clicking at the checkbox in the table header.
Comment #38
webchickNote: The OpenID thing was my bad, and is being fixed as a follow-up to #287178: Use hook_form_FORM_ID_alter() where possible (pending test coverage).
Comment #39
jrabeemer CreditAttribution: jrabeemer commentedsubscribe
Comment #40
mfer CreditAttribution: mfer commentedjQuery 1.3.1 regression.... $(document).ready() fires after images load in ie. Details at http://dev.jquery.com/ticket/3988 and http://stackoverflow.com/questions/477463/jquery-is-waiting-for-images-t...
Comment #41
jrabeemer CreditAttribution: jrabeemer commented@#40
A fix for the regression landed in svn.
http://dev.jquery.com/changeset/6170
Comment #42
webchickTum te tum.. :P
Comment #43
sirkitree CreditAttribution: sirkitree commentedNote: noticing that tabledrag is not working anywhere after upgrading to 1.3.1
Sorry, this may be the wrong place for this since I'm still using D6....
Comment #44
katbailey CreditAttribution: katbailey commentedWow, that tabledrag bug is an infuriating one to debug. Here's the error that gets logged to the console in Firebug:
Node cannot be inserted at the specified point in the hierarchy" code: "3
[Break on this error] this.parentNode.insertBefore( elem, this.nextSibling );
It's something to do with finding the target row (Drupal.tableDrag.prototype.findDropTargetRow) I guess. You can drag the rows right out of the DOM, they just disappear... The positioning seems to be off, i.e. where it's calculating mouse position and element offset. So maybe it's having trouble matching that up with what the target row should be. Or something. I don't know, I'm giving up for the night...
Comment #45
babbage CreditAttribution: babbage commentedSubscribe.
Comment #46
sunComment #47
katbailey CreditAttribution: katbailey commentedjQuery 1.3.2 is out (http://docs.jquery.com/Release:jQuery_1.3.2) and the tabledrag bug has not magically disappeared I'm afraid... :-(
Comment #48
RobLoachThis patch has jQuery 1.3.2, even though it says 1.3.1 in the CHANGELOG (whoops). Although the taxonomy page seems to be working, the block administration seems to be problematic. You can drag blocks off the table and they disappear from the DOM.
Comment #49
katbailey CreditAttribution: katbailey commentedI've done some more playing around with this and have established that the specific change in jQuery that is causing tabledrag to break is the domManip function. The old version of this function looked like this:
And here's the new version:
If you rebuild jQuery swapping out only this function (and changing the four functions that call it - append, prepend, before and after - so that they pass in the correct arguments) for its older counterpart from jQuery 1.2.6, the blocks tabledrag works perfectly.
The specific place in tabledrag.js where this becomes problematic is the swap function, i.e:
This code is basically calling .after() or .before() depending on the direction of the mouse movement, if I'm understanding it correctly.
I think we need quicksketch to look at this...
Comment #50
katbailey CreditAttribution: katbailey commentedActually, I just had a quick look in the jQuery bug tracker for anything related to domManip, and there's a related issue:
http://dev.jquery.com/ticket/4087
They ask "So how did you come up with this case? Is there some more complex code where you end up trying to insert a node after itself?"
If some more people could take a look at this, maybe we could plead our case to the jQuery folks that tabledrag.js in Drupal is indeed a more complex case that requires that old functionality back. Or maybe there's a change that can be made in tabledrag.js that will get around this...
Comment #51
Leeteq CreditAttribution: Leeteq commentedSubscribing.
Comment #52
quicksketchThanks katbailey for finding the source of the problem. With that information I found a work-around (but not a final solution!) that makes the blocks page work just fine.
The jQuery bug noted above (http://dev.jquery.com/ticket/4087) does indeed cause the disappearing table rows. However, it's not because we actually need to insertBefore/After on the same element that is being moved. It looks like what's happening is our math must be off somewhere, and tableDrag is trying to swap a row with itself, rather than the row above/below it.
So, this patch is a "bandaid" and definitely not what I'd recommend as our final solution. It simply puts an IF statement in the swap() method and keeps tableDrag from trying to swap a row with itself. The better solution would be to fix our math (where ever that is) instead, so the row doesn't try to swap with itself to begin with.
Comment #53
quicksketchHere's an improved version that I'd find acceptable. It looks like what I did previously actually was intentional, since we want the row to start moving even before the cursor leaves the row as it feels more responsive that way. However, we definitely do not want the row attempting to swap with itself. It seems this problem exists even in the D6 version, but it's just that jQuery was fine with attempting to swap an item with itself.
This version fixes the swapping problem. Since we're in tabledrag.js, I also converted some of our functions to use the jQuery dimensions plugin. There's no need for us to make our own calculation functions when jQuery provides (probably a much better one) for us.
Comment #54
RobLoachVery nice! Tested both the taxonomy and blocks administration pages in Firefox, Epiphany (Linux Gecko), Midori (Linux Webkit), Safari (Windows), IE6 (Windows) and Opera (Windows).
....... Still doesn't work in Lynx though (I kid, I kid).
Comment #55
RobLoachWait a second, I'm getting the following when uploading a file using Upload module:
Comment #56
quicksketchHmm, that's odd. I just noticed that exact error while reading through the jquery.form.js file. It's a hard-coded string, and apparently jQuery forms just doesn't think that buttons should be named submit, but only when uploading files. The author just recently moved the project to github so the entire SVN history is lost.
Note the problem is only when the name attribute is "submit",
<input type="submit" name="submit" value="Upload" />
, the value attribute should be fine.Comment #57
RobLoachUpdating to jQuery Form 2.21 seemed to fix the problem. The included provides what quicksketch had at #53, as well as a Dean Edwards' Packer base62 version of jQuery Form 2.21. Thanks a lot for narrowing it down, Nate!
Comment #58
jrabeemer CreditAttribution: jrabeemer commentedUpdated patch with version 1.3.2 with proper comment version.
I tested extensively using webchick's Javascript testing checklist (http://groups.drupal.org/node/5974) against IE6 & 7, Safari 3.2.1 Mac, Opera 9.64, and FF3.0.7.
This patch is RTBC.
Comment #59
jrabeemer CreditAttribution: jrabeemer commentedFor good measure, I just tested Google Chrome 1.0.154.48 in WinXP. Passed as well.
Comment #60
jrabeemer CreditAttribution: jrabeemer commentedMarking RTBC unless someone says otherwise..
Comment #61
mfer CreditAttribution: mfer commentedTested and looks good to me as well.
Comment #62
webchickCommittteeeedddd!!!! YAAAAYYYY!!!
This patch brought to you by delicious Chunky Monkey. :D
This needs to be documented in the upgrade docs.
Comment #63
jrabeemer CreditAttribution: jrabeemer commentedCVS $Id$ got lost in the the shuffle...
Comment #64
mfer CreditAttribution: mfer commentedThere's also an issue in Garland and FF that I'm working on. :)
Comment #65
RobLoachThe problem mfer is refering to is when you drag a row to the top and then drag it back to the bottom repeatedly. You end up with an empty row that keeps on growing by one pixel over and over again. See the attached screenshot.........
*cough* The solution is moving to jQuery UI Sortable ;-) .
Also still needs update docs.
Comment #66
jrabeemer CreditAttribution: jrabeemer commentedConfirmed in FF3 Mac. Space gradually grows as you drag on/off. Any ideas to what's causing that extra pixel?
Comment #67
katbailey CreditAttribution: katbailey commentedAs he mentioned in #53, quicksketch made other changes so as to use the positioning/offset stuff that's now built into jQuery itself - this looks to me like it could be a side-effect of that.
Comment #68
mfer CreditAttribution: mfer commentedThe issue is only in FF on garland and minelli. It seems to be related to the hidden tr that displays the message when nothing is in the region. If the display:none attached to that tr is removed or the tr is deleted from the page the problem goes away.
As with other fixes in garland and minelli for FF we might need some FF specific CSS to fix this. See https://developer.mozilla.org/en/CSS_Reference/Mozilla_Extensions for more details.
Comment #69
kewlguy CreditAttribution: kewlguy commentedI tried installing this patch in my drupal 6.10 with Gallery2 website and the patch had many errors on install. I dropped the patch into my drupal misc folder and then ran it through putty. I could have sworn that the page I got the patch from was for 6.x though I note now that this version is for the 7.x branch?
Here is a sampling of what I received through putty:
xxxxxxxx@handymanreality.com [~/public_html/misc]# patch -b < jquery_19.patch
patching file ahah.js
Hunk #1 succeeded at 181 (offset -3 lines).
patching file jquery.form.js
Hunk #1 FAILED at 2.
Hunk #2 FAILED at 10.
2 out of 2 hunks FAILED -- saving rejects to file jquery.form.js.rej
patching file jquery.js
Hunk #1 FAILED at 1.
1 out of 1 hunk FAILED -- saving rejects to file jquery.js.rej
patching file tabledrag.js
Hunk #1 succeeded at 203 (offset -3 lines).
Hunk #2 FAILED at 483.
Hunk #3 succeeded at 501 (offset -2 lines).
Hunk #4 succeeded at 519 (offset -3 lines).
Hunk #5 succeeded at 533 (offset -2 lines).
1 out of 5 hunks FAILED -- saving rejects to file tabledrag.js.rej
patching file teaser.js
Hunk #1 FAILED at 8.
Hunk #2 FAILED at 85.
2 out of 2 hunks FAILED -- saving rejects to file teaser.js.rej
To get myself out of the situation I simply downloaded a fresh copy of 6.10 and replaced the misc directory with the downloaded one.
Of course I still have trouble with scripts that are slow or not responding from my browser usually on the build/modules pages as well as the permissions pages.
Comment #70
drewish CreditAttribution: drewish commentedkewlguy, this patch is for D7 only. you're going to have zero luck getting this to work with D6.
Comment #71
mfer CreditAttribution: mfer commentedAn issue we have is that on the blocks page, when garland is your theme, and you're using firefox and you move blocks to the top of a region there is a 1px growth each time. It's rather strange.
Attached is a 'hack' (at least I think it is) which switches from CSS display to visibility for FF. With this change it works correctly. But, it doesn't fix the root of the problem... what ever that may be (I'm still not sure).
I would prefer a better fix for this problem. The 1px issue seems to surround the hidden row that holds the message when there are no blocks in the region.
Comment #72
Steven Merrill CreditAttribution: Steven Merrill commentedOne initial note: turning border-collapse off and on in Firebug kills the ghost space in FF3/Mac.
Comment #73
mfer CreditAttribution: mfer commentedIn this patch I turned off border-collapse, set the borders to 0 for the table, and added the borders from the rows to the td elements in the rows so they visually look the same.
Thoughts?
Comment #74
mfer CreditAttribution: mfer commentedOf course, the border-spacing property is not supported by IE....
Comment #75
mfer CreditAttribution: mfer commentedThis patch fixes it. Just adjusts the borders...
Comment #76
webchickGreat! I confirm this gets rid of that silly border problem. However, "sometimes" I get this sort of 3/4 shading thing:
I'm really trying to remember what I did to accomplish this. I'm thinking it was something like drag a block up two spaces, then back down one, but I really don't know. :(
Could you take a look and see if you get the same thing?
[edit: now with proper input format. ner.]
Comment #77
mfer CreditAttribution: mfer commentedI was able to duplicate the issue webchick noted in #76. I was, also, able to duplicate it in drupal 6 on a live site in both safari and FF. See http://skitch.com/mattfarina/befwj/drupal-6-blocks-sorting-visual-bug. So, that bug isn't related to this issue.
Turns out the problem was in the tabledrag.js code. The attached patch fixes the growth issue in FF as well as a issue in Safari/Chrome/Webkit Head that caused some 1px jumping. I tested in safari, chrome, webkit head, FF, IE6, and IE7.
Comment #78
mfer CreditAttribution: mfer commentedFiled an issue for the bug webchick noted in #76 at #403028: Block page color issue
Comment #79
Steven Merrill CreditAttribution: Steven Merrill commentedTested on IE6/Win, IE7/Win, FF2/Mac, FF3/Mac, and Opera9.63/Mac.
Drupal 7 Garland now behaves exactly the same as Drupal 6 Garland. There are still two issues in Garland itself, which mfer is going to file issues for.
The first existing issue is where the top item in FF will always become too tall after a drag. It also exists in a bone-stock 6.10 install, so we're no worse off: http://skitch.com/00sven/bekfq/blocks-localhost .
There continues to also be a strange issue with the grippie and the top border on IE6 / 7, but again, this appears in both Drupal 6 and 7 and thus is not new: http://skitch.com/00sven/bekji/boot-camp-partition.
Rock on!
Comment #80
Steven Merrill CreditAttribution: Steven Merrill commentedOy. Also tested on Safari3/Mac.
Comment #81
webchickAwesome, thanks!
Committed with some minor whitespace fixes and comment wrapping.
Comment #82
mfer CreditAttribution: mfer commentedThanks Steven. The two issues he noted are at #404298: Incorrectly sized blocks on the blocks sorting UI and #404302: Improve tabledrag grippie CSS (and fix an IE7 alignment issue)
Comment #83
mfer CreditAttribution: mfer commentedComment #84
webchickOh wait. Except it still needs upgrade documentation. Back to CNW.
Comment #85
RobLoachAnything that's missing here?
We should also evaluate the possible replacement of Drupal.behaviors with jQuery.live(). mfer created #404902: Replace Drupal.behaviors with jQuery.live().
Comment #87
mfer CreditAttribution: mfer commentedDocumentation added.
Comment #88
sunNow that I have all JavaScript freaks in one issue - eat this: #418304: AJAX/AHAH requests are hitting server twice ;)
Either we have a critical JS bug in core or jQuery 1.3 contains this critical bug.
Comment #89
mfer CreditAttribution: mfer commentedThere was a change in the ahah code for drupal. The change was:
But, I don't see this code causing ahah to call back twice. Can you provide steps to reproducing the problem?
Comment #91
priyanka.salunke CreditAttribution: priyanka.salunke commentedhey can u tell me to upgrade jquery to jQuery Update 2.x (which includes jQuery 1.3.x)
i got patch files bt i dnt hv any idea where to plce this code to which file i hv to update
.which file i hv to update plz give me a detail step by step procedure to update my jquery
Comment #92
salihcenap CreditAttribution: salihcenap commented53: jquery_13_upgrade.patch queued for re-testing.