jquery.js: Fix CVS ID, remove SVN tags

Sounds like http://drupal.org/node/224442#comment-812153 and http://dev.jquery.com/ticket/2386.

It seems like the source of this issue is in jquery 1.2.3 .click event, but I'm not aware about any workaround.

If you find a working solution please post.

There seems a bugfix in jQuery 1.2.4b. I have updated myself and I'm no more able to repro this issue.

You can download jQuery UI (http://jqueryjs.googlecode.com/files/jquery.ui-1.5b4.zip) and get the new jQuery 1.2.4a file from there. Then make a backup of you "misc/jquery.js" and replace it with the newer version. I don't know where you can find a compressed version of 1.2.4a file.

Moving to core as this is not a bug in GA. I hope this new jQuery version get's into D6 very soon...

Great... we have a 1.2.4 release and this seems to be the bug http://dev.jquery.com/ticket/2613.

Patch attached to update jQuery to the next level. Please test...

Also affected by this bug: #224442: Firefox only: Google Analytics - Lightbox2 - jQuery 1.2.3 conflict

Are there API changes? http://docs.jquery.com/Downloading_jQuery still says "Current release 1.2.3", so it is hard to get to the changelog or whatever.

This seems to be a bugfix only release. http://docs.jquery.com/Release:jQuery_1.2.4 contains only a link to the list of fixed bugs http://dev.jquery.com/report/27

jQuery 1.2.5 is out now: http://docs.jquery.com/Downloading_jQuery

New patch attached.

This is a bugfix release for 1.2 see http://docs.jquery.com/Release:jQuery_1.2.5

I don't see any reason why this can't go into D7 then be backported, so moving it there.

I only hope this goes in before Drupal 6.3 get's released.

It'll go in quicker (at all, in fact) if it's tested on all browsers using the items in this list. Hass, have you done tested the patch? Care to share the results?

Well, that bug list links to http://dev.jquery.com/ticket/2249 among others. This and some others are classified as "enhancements", so I'd not classify this release as bugfix only. This one is clearly a new feature, and was only a quick find to look up. Anyway, let's make sure it does not break any of the core stuff, and then we can see whether this goes into D6.

@Gabor: Well there might be new features in jQuery and i don't care about them, but today nearly every JS popup is broken in Firefox. This is a serious bug and affects many contrib modules! I'm aware about ~5 cases discussing the same issue. Lightbox was able to add a workaround, but the bug listed above seems not solvable without the update :-(.

I've got many complains that Google Analytics is broken or better to say GA break other things and GA is not the source of this issue and cannot workaround :-(.

@catch: thank you for the above link, i will try to test all of this, but I'm limited to Windows and finally i cannot set my own patch to RTBC...

No, but you can report your testing results of each one of those items in IE 6 / 7, which would be a huge help.

hass: whatever bugs this solves is not an argument, if there are new bugs introduced. The role of the current state "code needs review" is to prove it does not introduce new bugs and API changes which break existing scripts.

@Gabor: I know this... i don't like to introduce new bugs i'd only like to get this popup bug fixed and i do not care about "new feature" nobody is using today. New jQuery features shouldn't hold up the patch if it fixes other critical bugs and the API's haven't changed.

Testresults:
Windows, Firefox 2.0: All works as expected

Untested: OpenID (i have no account for testing)

So these browsers left to test then:

IE6/7 (Windows)
Opera (any platform)
Safari 2 (Mac) 3 (Mac/Windows)
Firefox/Mac
Konqueror usually gets a run through as well.

Something with farbtastic.js seems to be broken in IE6. The circle is no more working as before... i cannot select all colors and i cannot fix myself. Looks like a mouse positioning issue

There are no release notes yet for jQuery 1.2.6. But given this is a bugfix release, is there any reason to believe that this would not be a safe patch to commit? For all I know, it actually fixes a bug in Drupal.

Nearly all bugfix releases included some new features / changes in existing stuff in the past, so we always needed to retest every point upgrade when the jQuery update issues were in full force. That makes me a bit skeptical to believe that we should be just fine without testing this.

Gábor, Hass, could you came up with a test plan for this? I'm sure most of us can help testing that, but we need to know what to test and report.

@Damien: http://groups.drupal.org/node/5974#javascript has the list of all of the places in core that use JS (or at least the ones we documented ;)). Each of these needs to be tested in all major browsers (IE 6+, FF 2+, Safari 2+, Opera whatever-the-latest-version-is, and Konqueror whatever-the-latest-version-is) on various platforms (Windows, Mac, Linux).

Does anyone have a line to the jQuery community to have an idea if there's going to be yet another release in the next couple days? Since this sort of testing takes a LOT of time, it'd be nice to only have to do it once, not 3+ times in a week. :\

#23 is still broken in 1.2.6.

@webchick - according to the 1.2.6 release notes, "This is the next release immediately following jQuery 1.2.3. Releases 1.2.4 and 1.2.5 were skipped (1.2.4 was built incorrectly, rendering it effectively identical to 1.2.3, and 1.2.5 was missing a patch)."

Marking to needs work for farbtastic breakage.

Someone familiar with farbtastic and able to take a look, please?

There is one person familiar with Farbtastic, and he's no longer in the Drupal community. So, if you want this in, get nice and cushy with Firebug. ;)

I'm not at all familiar with farbtastic, but I copied changed functions from the jQuery 1.2.6 release docs then did a ctrl-F on farbtastic - likely candidates are .toggle() and/or .attr() from this bit of color.js:

    // Add lock.
    var i = inputs.length;
    if (inputs.length) {
      var lock = $('<div class="lock"></div>').toggle(
        function () {
          $(this).addClass('unlocked');
          $(hooks[i - 1]).attr('class',
            locks[i - 2] && $(locks[i - 2]).is(':not(.unlocked)') ? 'hook up' : 'hook'
          );
          $(hooks[i]).attr('class',
            locks[i] && $(locks[i]).is(':not(.unlocked)') ? 'hook down' : 'hook'
          );
        },
        function () {
          $(this).removeClass('unlocked');
          $(hooks[i - 1]).attr('class',
            locks[i - 2] && $(locks[i - 2]).is(':not(.unlocked)') ? 'hook both' : 'hook down'
          );
          $(hooks[i]).attr('class',
            locks[i] && $(locks[i]).is(':not(.unlocked)') ? 'hook both' : 'hook up'
          );
        }
      );
      $(this).after(lock);
      locks.push(lock);
    };

Sounds like bad news :-(

Hm... the locking and unlocking of colors is working as i know. It's the positioning inside the color circle that is broken. You point to the right side of the circle but the marker moves on the left...

@webchick: I have no issues with Firefox... it's only IE and Firebug doesn't help here...

http://getfirebug.com/lite.html

The problem seems to be in farbtastic.js in the function fb.widgetCoords which is reporting different x coordinate position between firefox and IE6. It looks like the x coordinate is becoming a negative number for some reason. At least this is where I traced it. The problem could be with the event being passed in here.

Ugh, anyone know any javascript masters who want to work on this ;-)?

In IE 6 the farbtastic.js function widgetCoords has event.offsetX set to undefined in 1.2.6 and a value in 1.2.3. This difference seems to be the issue. The value there in 1.2.3 is what the logic calculates in browsers not named IE for the case it's set to undefined. IE calculates something different than firefox. What firefox calculates is the same as event.offsetX (in 1.2.3) that is passed in.

This is as far as I'm tracing this today. Anyone want to pick up where I left off.

This patch updates to jQuery 1.2.6 and farbtastic is updated to work in IE. I tested in in Firefox 2 and Safari on a Mac and Firefox 2 and IE 6 under Windows.

The change happened because the event object coming didn't have a relative offset (offsetX and offsetY) to the parent element for IE in 1.2.6 like there was under 1.2.3. It seems the event object changed. The math calculation in place (based on the absolute position in the page) if the offests weren't defined (for browsers like firefox) didn't work right in IE.

My solution was to calculate the offset if it wasn't there with

  if ( typeof event.offsetX == 'undefined' && typeof event.offsetY == 'undefined' ) {
      var offset = $(event.target).offset(false);
      event.offsetX = event.pageX - offset.left;
      event.offsetY = event.pageY - offset.top;
  }

and remove the absolute method making all browsers calculate it relatively.

I'm not sure if this is the best way to go about it. Thoughts?

The change happened because the event object coming didn't have a relative offset (offsetX and offsetY) to the parent element for IE in 1.2.6 like there was under 1.2.3. It seems the event object changed.

Well, these are the kind of changes because of which some (JS dependent) contrib modules might need pre-6.3 and post-6.3 versions. It would be great to see if this was done for a bugfix, or there are possibly other such API changes lurking in the code base, waiting to unleash themselves if we throw the code with a Drupal point release on contrib / production sites.

I went digging through drupal 6 contrib and it doesn't look like any modules are taking advantage of the of the event offset. In the case a module wants to use it the snippet of code in #41 will add it to the event.

Thank you for the farbtastic fix. I've also tested in IE6 and it works for me.

OK, who tested with the full JS test plan? Any remaining issues identified?

Finally got around to testing this out against the test suite at http://groups.drupal.org/node/5974#javascript plus adding additional poll items.

I tested on a mac in Safari 3, Firefox 2 and 3rc2, and Opera 9.27 and 9.5b2.

Tested on a pc in IE 6 & 7, Firefox 2 and 3rc2, and Opera 9.27 and 9.5b2.

The only issue I noticed was for Opera 9.27 in file attachments on a node. When you try to attach a file an error is returned. But, this is not a new error and is in drupal 6 already with Opera 9.27.

I should mention that the patch in #41 is against drupal 6 as is all this testing. It was against the latest from the drupal 6 branch.

Any chance of getting this into drupal 6.3?

Here is a patch against the latest head for D7. I didn't run it through all the testing rigors but it seems to be working.

No patch. :(

oops. here is the patch against D7. The D6 patch is #41.

Cleaning mfer's patch #49.

I committed the patch to CVS HEAD. That should give it some more exposure, and could be a good test case before we decide to commit it to D6 as well.

I have no experience with Drupal core development or farbtastic and I can't do much more than offer some info. It looks like this thread is the place to post this note. We're running an up to date D6.2 and getting an error from the following code:

;// $Id: farbtastic.js,v 1.4 2007/06/01 09:05:45 unconed Exp $
// Farbtastic 1.2

jQuery.fn.farbtastic = function (callback) {
$.farbtastic(this, callback);
return this;
};

jQuery.farbtastic = function (container, callback) {
var container = $(container).get(0);
return container.farbtastic || (container.farbtastic = new jQuery._farbtastic(container, callback));
};

The error is on that last return, " 'farbtastic' is null or not an object ". It's consistent with IE7 on three different systems here. The date 2007/06/01 scares me because this is a current 6.2 core. I'm guessing it's just this code segment that's from 2007.

I'll be happy to tweak code or do other diagnostics as required.
HTH

@starbuck - Try the jQuery Update module and specifically the current nightly dev at http://drupal.org/node/270693. This should be stable enough to release and it includes 1.2.6 with a fix for farbtastic. Just be sure the version you have in core is 1.2.3. It will do a comparison or jquery files and only swap out javascript files if it detects the one in core is older.

We need to remember to update the jQuery section of COPYRIGHT.txt, assuming this patch does not do so.

keith: no there is no update required.

Yes, sorry, after a review, I see that COPYRIGHT.txt does not mention a specific version number. Thanks hass.

Based on testing by mfer in #46, I have committed his patch to Drupal 6. His new jquery.js lacked a few Drupal modifications we have:

- The CVS Id tag at the top
- The SVN $Date and $Rev tags being de-tagged with the $-s removed.

So I made these modifications to his patch. Committed to Drupal 6!

These two things are still to be made to the Drupal 7 commit. So Patch needs work for Drupal 7.

Nevertheless I think an jQuery 1.3 will go into D7... here is a patch :-)

Error persists on $.farbtastic line which is cited as line# 5751. I can load anything that's recommended to try another fix... Thanks

// $Id: farbtastic.js,v 1.1 2008/06/15 04:07:15 webchick Exp $
// Farbtastic 1.2

jQuery.fn.farbtastic = function (callback) {
  $.farbtastic(this, callback);
  return this;
};

@Starbuck I see webchicks name on that farbtastic.js file. Are you refering to core development (this thread) or to the jquery_update module. If it's the latter please post an issue over at http://drupal.org/project/issues/jquery_update with details about your browser (with version), drupal/module version, etc. Oh, and specifics of the error and what you see the error in.

@Starbuck see http://drupal.org/node/275740 for details.

I can confirm this working on drupal 5.7

Any chance of using the minified jquery which can be gzipped instead of the packed version which requires the browser to decompress on every page load?

I upgraded to v6.3 and the problem persists.

Summary: First I tried the vanilla 6.3. Then I applied the latest jQuery Update. Then I realized the latest jQuery Update is behind core so I disabled the update. Then I tried to patch jquery.js with the file that hass provided on June 25th. No joy all day. From what I can tell the code affecting this has not changed at all in the last couple months.

Detail: The error I'm seeing starts in jquery.themebuilder.js which has this line:
$('#colorpicker').farbtastic('#edit-color');
That calls to misc/farbtastic/farbtastic.js where the error is not thrown by this line as I originally thought:
return container.farbtastic || (container.farbtastic = new jQuery._farbtastic(container, callback));
It's the call right before that:
var container = $(container).get(0);
That starts with container being an object and comes back with container being undefined. I can see the code:
- executing through makeArray;
- it returns with an empty object;
- setArray turns it into a single element array;
- the get function shows num=0, not undefined;
- so get is supposed to return this[0];
- this is definitely an array with a single element;
but when we get back to that var container assignment, container is undefined. And... Bewm.

Misc:

I can follow the code but I don't pretend to understand what it's doing since I'm not familiar with farbtastic or jQuery or anything else going on except for some basics of how Drupal finds the right code to execute. I don't have a javascript debugger that shows the detail of the objects, only that they are in fact arrays or objects. The key factor is that the get function is returning an object but when it comes out of the pipe it's undefined. Go figure.

There is a confusing variety of dates and release numbers. I recommend the README's be updated and that the module pages also be updated with a brief summary and/or cross-reference. Someone can easily install the jQuery "Update" and essentially downgrade their release. I'll be happy to try new patches for any of this - we're still in development here, not production.

Thanks for ongoing assistance here.

@Starbuck: jquery.themebuilder.js is not part of core. Use a vanilla 6.3 and you have jQuery 1.2.6 build in. Please disable all custom modules and then enable one by one to figure out the buggy module. I would start disabling themebuilder module first - if there is an error as you found out.

I disabled themebuilder and the problem went away. Then another small and completely unrelated problem in another module was able to raise its little head.

Of course the error will go away if I disable the module that calls that code. I would hope that the goal isn't to stop executing problem code, it's to find out what's wrong with it. Finding triggers is important, but I already pointed to the line of code that starts the chain and provided some idea about the values passed down the line. Can I do anything else to help?

Thanks for the note, hass.

Please open a bug against themebuilder module.

@Webchick: patch in http://drupal.org/node/256285#comment-897078 need to go into D7 first :-).

Oops. :) Thanks!

jQuery 1.2.6 is included with the Drupal 5. It is very well supported in the jQuery community.
The tricky step with this module is that you will need to replace the jQuery.js file in core with the jquery.
It includes both 0.1 and 1.1 compat.
----------------------------------------

Marvin

widecircles

These are the kind of changes because of which some contrib modules might need pre-6.3 and post-6.3 versions. It would be great to see if this was done for a bugfix.
----------------------------------------------------
pandreson

widecircles

I would love to know if any modules needed a pre-6.3 and a post-6.3 version. Outside of farbtastic which took advantage of something that nothing else in all of drupal cvs did I couldn't find any javascript code that would be impacted. I could have easily missed something and if so I'd love to see the place where it happened.

Looking at misc/jquery.js on my D7 site:

 * jQuery 1.2.6 - New Wave Javascript

I mark it "fixed" based on this.

Patch in 58 haven't been committed yet!

That is hardly critical or a bug - comment changes only.

Yeah... when the case started it was a critical bug in jquery onclick events that broke many modules and nobody changed the status after this part was fixed. :-)

As this is an external library, the code is packed, and we probably won't ever modify the code of jQuery, I'd say: leave it as is. All a user might want to learn from this file is the jQuery version, which is already visible/readable.

I think the patch in #58 haven't been committed yet and therefore the below lines are not yet fixed in CVS.

- * $Date: 2008/06/12 19:49:10 $
- * $Rev: 5685 $
+ * Date: 2008-05-24 14:22:17 -0400 (Sat, 24 May 2008)
+ * Rev: 5685

What's wrong with those lines?

CVS is taking over the original SVN tags from upstream, and the mandatory $Id$ tag is absent.

The patch to commit is #58.

Committed to CVS HEAD. Thanks.

D6 is already correct, setting status to fixed.