This is the particular behaviour that has been causing problems in IE8

/**
 * Sends a 'formUpdated' event each time a form element is modified.
 */
Drupal.behaviors.formUpdated = {
  attach: function (context) {
    // These events are namespaced so that we can remove them later.
    var events = 'change.formUpdated click.formUpdated blur.formUpdated keyup.formUpdated';
    $(context)
      // Since context could be an input element itself, it's added back to
      // the jQuery object and filtered again.
      .find(':input').andSelf().filter(':input')
      // To prevent duplicate events, the handlers are first removed and then
      // (re-)added.
      .unbind(events).bind(events, function () {
        $(this).trigger('formUpdated');
      });
  }
};

I changed it to


/**
 * Sends a 'formUpdated' event each time a form element is modified.
 */
Drupal.behaviors.formUpdated = {
  attach: function (context) {
    // These events are namespaced so that we can remove them later.
    var events = 'change.formUpdated click.formUpdated blur.formUpdated keyup.formUpdated';
    $(context).undelegate(':input', events);
    $(context).delegate(':input', events, function () {
        $(this).trigger('formUpdated');
      }
    );
  }
};

This seems to improve the performance a lot on that page in IE8

Comments?

Comments

catch’s picture

Version: 7.9 » 8.x-dev
Component: forms system » javascript
Issue tags: +Needs backport to D7

Moving this to the right queue.

nod_’s picture

That's the way to go. Needs to change across all of core too. I think the jQuery method changed in the newer version D8 is using (will be using soon).

gitesh.koli’s picture

StatusFileSize
new1.05 KB

Please find attached the patch for 7.x version

attiks’s picture

@gitesh.koli needs to be fixed for D8 first, can you make a patch?

gitesh.koli’s picture

StatusFileSize
new1.04 KB

D8 version of the patch. (using on and off function instead of delegate and undelegate)

nod_’s picture

Status: Active » Needs review

Status: Needs review » Needs work

The last submitted patch, 1515734-d8+patch-1.patch, failed testing.

gitesh.koli’s picture

StatusFileSize
new882 bytes

patch against the latest D7 version.

gitesh.koli’s picture

StatusFileSize
new917 bytes

D8 version of the patch

JamesK’s picture

Status: Needs work » Needs review

test bot

gitesh.koli’s picture

#8: 1515734-form-js-changes-d7-1.patch queued for re-testing.

JamesK’s picture

Status: Needs review » Needs work

This should not be removed:

      // To prevent duplicate events, the handlers are first removed and then
      // (re-)added.
nod_’s picture

I'd like this to do some more work and trigger the event on the actual <form> with a custom parameter containing a DOM node of the changed form element.

This issue would more or less be what's needed for step 1 of #1636992: form.js' formUpdated event is unreliable/incomplete but we need to check that the form has actually been updated before firing the event. As it is now it's pretty useless.

wim leers’s picture

Title: form.js slows down the page processing when there are too many elements in IE8 » form.js' 'formUpdated' behavior is slow because it binds to each element rather than delegating
Issue tags: +JavaScript, +front-end performance
wim leers’s picture

Status: Needs work » Needs review
Issue tags: -JavaScript, -Needs backport to D7, -front-end performance

#9: 1515734-form-js-changes-d8-1.patch queued for re-testing.

Status: Needs review » Needs work

The last submitted patch, 1515734-form-js-changes-d8-1.patch, failed testing.

nod_’s picture

Issue summary: View changes
Status: Needs work » Closed (duplicate)