In commerce_kickstart_search.js the following line is part of the behavior added by the module.

$('.facetapi-checkbox').live('click', function(e) {

The live function has been deprecated as of jQuery 1.7 and completely removed as of version 1.9. In themes that override to newer versions of jQuery, or sites that use the jQuery Update module, this generates an error.

If backwards compatibility is essential then bind can be used instead (which in turn calls on).

If being current is more important, the on function can be used in combination with the jQuery Migrate plugin, so that when the plugin is dropped (i.e. CK no longer supports the older versions) no changes are needed in the actual code.

I don't know what the policy is regarding backwards compatibility, but jQuery version is always an issue with new themes (hence the jQuery Update module and theme overrides).

So, just throwing this out there...

CommentFileSizeAuthor
#11 commerce_search_error-2106687-11.patch684 bytesmglaman
#8 commerce_search_error-2106687-8.patch682 bytesmglaman
#3 commerce_kickstart-search_js_jquery_latest-2106687-3.patch770 bytesbc
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

gsbc’s picture

gsbc CreditAttribution: gsbc commented
Title: Commerce search error with newer jquery versions » Commerce search error with newer jquery versions (above 1.9)
lsolesen’s picture

lsolesen CreditAttribution: lsolesen commented
Category: Support request » Feature request
Priority: Minor » Normal
Issue summary: View changes
Status: Active » Postponed (maintainer needs more info)

@gsbc Could you try creating a patch for the change, that you suggest?

bc’s picture

bc CreditAttribution: bc commented
FileSize
commerce_kickstart-search_js_jquery_latest-2106687-3.patch770 bytes

I forward-ported in the $.live workaround from jquery-migrate... works like a charm.

bc’s picture

bc CreditAttribution: bc commented
Status: Postponed (maintainer needs more info) » Needs review
urecheatu007’s picture

urecheatu007 CreditAttribution: urecheatu007 as a volunteer commented

I just tested this patch on a fresh install of commerce kickstart with jquery update (1.10) and it seems to be working fine.

dthorberg’s picture

dthorberg CreditAttribution: dthorberg commented

Agreed I also just patched this on a new copy of D7 with the latest patch. Fixed my javascript woes. (Version 7x.2.25)

mglaman’s picture

mglaman
Primary language English
Location WI, USA
CreditAttribution: mglaman at Centarro commented
Status: Needs review » Needs work
Issue tags: +sprint 
+++ b/modules/commerce_kickstart/commerce_kickstart_search/commerce_kickstart_search.js
@@ -1,5 +1,14 @@
+$.fn.live = function( types, data, fn ) {
+  if ( oldLive ) {
+    return oldLive.apply( this, arguments );
+  }
+  $( this.context ).on( types, this.selector, data, fn );
+  return this;
+};

If this can be resolved by just using bind then I'd rather do that, then this workaround.

mglaman’s picture

mglaman
Primary language English
Location WI, USA
CreditAttribution: mglaman at Centarro commented
Version: 7.x-2.9 » 7.x-2.x-dev
Status: Needs work » Needs review
FileSize
commerce_search_error-2106687-8.patch682 bytes

After reading this: http://elijahmanor.com/differences-between-jquery-bind-vs-live-vs-delega..., I think we can just change live to on method.

Here is the PR https://github.com/commerceguys/commerce_kickstart/pull/120

bc’s picture

bc CreditAttribution: bc commented

The "oldLive" patch keeps compatibilty with jQuery versions < 1.9.

mglaman’s picture

mglaman
Primary language English
Location WI, USA
CreditAttribution: mglaman at Centarro commented
Status: Needs review » Needs work

Ok, I'm moving back to needs work. Totally missed .on() was added in 1.7 and Drupal ships with 1.4. My one thought is "we're not the only ones with this issue." The patch in #3 creates our own work around. I'd rather investigate what other contributed projects are doing, too.

mglaman’s picture

mglaman
Primary language English
Location WI, USA
CreditAttribution: mglaman at Centarro commented
Status: Needs work » Needs review
FileSize
commerce_search_error-2106687-11.patch684 bytes

I'd rather just stick with bind rather than other workarounds, if we can.

Link to Travis build: https://travis-ci.org/commerceguys/commerce_kickstart/builds/70471380

mglaman’s picture

mglaman
Primary language English
Location WI, USA
CreditAttribution: mglaman at Centarro commented

Tests pass!

  • mglaman committed 4cfee3a on 7.x-2.x 
    Merge pull request #120 from mglaman/2106687-search-jquery-live

Issue #...
  • mglaman committed 5080ec6 on 7.x-2.x
    Issue #2106687 by bc: Commerce search error with newer jquery versions (...
mglaman’s picture

mglaman
Primary language English
Location WI, USA
CreditAttribution: mglaman at Centarro commented
Status: Needs review » Fixed

Status: Fixed » Closed (fixed)

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