While debugging code through Firebug, I noticed that one click() event was added to each BEF checkbox after each AJAX request (filtering the AJAX-enabled view). So after filtering the view 10 times, there will be 10 click() handlers attached to each checkbox.

To reproduce, create a view, enable AJAX on it, add some exposed filters as checkboxes with BEF, then inspect one of the checkboxes with Firebug + FireQuery before and after applying filters.

The issue comes from this snippet that runs every time the page is updated (so after each AJAX request) since it's attached to Drupal.behaviors:

// Add highlight class to checked checkboxes for better theming
$('.bef-tree input[type="checkbox"], .bef-checkboxes input[type="checkbox"]')
// Highlight newly selected checkboxes
.click(function() {
this.checked
? $(this).parent().addClass('highlight')
: $(this).parent().removeClass('highlight');
})
// Highlight ones that are checked on page load
.filter(':checked').parent().addClass('highlight')
;
};
?>

The fix is straightforward, patch incoming.

CommentFileSizeAuthor
#1 js_leak.1473308-1.patch516 bytesmdupont
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

mdupont’s picture

mdupont
Primary language French
Location Switzerland
CreditAttribution: mdupont commented
FileSize
js_leak.1473308-1.patch516 bytes

Oops, here is the snippet:

// Add highlight class to checked checkboxes for better theming
    $('.bef-tree input[type="checkbox"], .bef-checkboxes input[type="checkbox"]')
      // Highlight newly selected checkboxes
      .click(function() {
        this.checked
          ? $(this).parent().addClass('highlight')
          : $(this).parent().removeClass('highlight');
      })
      // Highlight ones that are checked on page load
      .filter(':checked').parent().addClass('highlight')
    ;
  };

Patch attached that ensures only 1 instance of this event will be attached to each checkbox.

mdupont’s picture

mdupont
Primary language French
Location Switzerland
CreditAttribution: mdupont commented
Status: Active » Needs review
mikeker’s picture

mikeker CreditAttribution: mikeker commented
Status: Needs review » Postponed (maintainer needs more info)

Sorry for the delay getting to this issue.

Does this still happen with the latest -dev release? The current JS uses the following selector:

$('.form-checkboxes.bef-select-all-none:not(.bef-processed)');

which should eliminate the need for a .not('bef-processed')

mdupont’s picture

mdupont
Primary language French
Location Switzerland
CreditAttribution: mdupont commented
Category: bug » task
Priority: Normal » Minor
Status: Postponed (maintainer needs more info) » Active

Indeed, the JS code in the latest dev makes sure that the click event will only be added once.

However I see that the 7.x version is still using the '.not(".bef-processed").addClass("bef-processed")' pattern when D7 bundles the jQuery Once plugin that is the preferred way to do this, see http://drupal.org/node/171213 section "Javascript Behaviors".

Switching issue to a minor task since it's mainly a code quality improvement to use .once() in the parts of the code where it makes sense.

mikeker’s picture

mikeker CreditAttribution: mikeker as a volunteer commented
Issue summary: View changes
Status: Active » Closed (won't fix)

Issue queue cleanup... I'm sorry for taking so long to get back to this issue!

@mdupont, you are completely correct, once() is the better way to handle that. And the 7.x-3.x branch has moved to that pattern. Closing as "won't fix" mostly because it has already been fixed the current branch and the 6.x branch is near it's end-of-life.

Thanks.