Commit ae4177b8 authored by webchick's avatar webchick

Issue #2159965 by Wim Leers, jessebeach: Fix two memory leaks + subtle bugs in...

Issue #2159965 by Wim Leers, jessebeach: Fix two memory leaks + subtle bugs in Edit's JS (discovered by working on the Backbone upgrade in the D7 backport).
parent ebd7e8fb
...@@ -183,7 +183,9 @@ Drupal.edit.editors.form = Drupal.edit.EditorView.extend({ ...@@ -183,7 +183,9 @@ Drupal.edit.editors.form = Drupal.edit.EditorView.extend({
fieldModel.set('htmlForOtherViewModes', response.other_view_modes); fieldModel.set('htmlForOtherViewModes', response.other_view_modes);
// Finally, set the 'html' attribute on the field model. This will cause // Finally, set the 'html' attribute on the field model. This will cause
// the field to be rerendered. // the field to be rerendered.
fieldModel.set('html', response.data); _.defer(function () {
fieldModel.set('html', response.data);
});
}; };
// Unsuccessfully saved; validation errors. // Unsuccessfully saved; validation errors.
......
...@@ -81,9 +81,11 @@ Drupal.edit.EntityModel = Backbone.Model.extend({ ...@@ -81,9 +81,11 @@ Drupal.edit.EntityModel = Backbone.Model.extend({
var to = state; var to = state;
switch (to) { switch (to) {
case 'closed': case 'closed':
this.set('isActive', false); this.set({
this.set('inTempStore', false); 'isActive': false,
this.set('isDirty', false); 'inTempStore': false,
'isDirty': false
});
break; break;
case 'launching': case 'launching':
...@@ -103,18 +105,9 @@ Drupal.edit.EntityModel = Backbone.Model.extend({ ...@@ -103,18 +105,9 @@ Drupal.edit.EntityModel = Backbone.Model.extend({
case 'committing': case 'committing':
// The user indicated they want to save the entity. // The user indicated they want to save the entity.
// For fields already in a candidate-ish state, trigger a change var fields = this.get('fields');
// event so that the entityModel can move to the next state in
// committing.
this.get('fields').chain()
.filter(function (fieldModel) {
return _.intersection([fieldModel.get('state')], Drupal.edit.app.readyFieldStates).length;
})
.each(function (fieldModel) {
fieldModel.trigger('change:state', fieldModel, fieldModel.get('state'), options);
});
// For fields that are in an active state, transition them to candidate. // For fields that are in an active state, transition them to candidate.
this.get('fields').chain() fields.chain()
.filter(function (fieldModel) { .filter(function (fieldModel) {
return _.intersection([fieldModel.get('state')], ['active']).length; return _.intersection([fieldModel.get('state')], ['active']).length;
}) })
...@@ -123,7 +116,7 @@ Drupal.edit.EntityModel = Backbone.Model.extend({ ...@@ -123,7 +116,7 @@ Drupal.edit.EntityModel = Backbone.Model.extend({
}); });
// For fields that are in a changed state, field values must first be // For fields that are in a changed state, field values must first be
// stored in TempStore. // stored in TempStore.
this.get('fields').chain() fields.chain()
.filter(function (fieldModel) { .filter(function (fieldModel) {
return _.intersection([fieldModel.get('state')], Drupal.edit.app.changedFieldStates).length; return _.intersection([fieldModel.get('state')], Drupal.edit.app.changedFieldStates).length;
}) })
...@@ -192,18 +185,56 @@ Drupal.edit.EntityModel = Backbone.Model.extend({ ...@@ -192,18 +185,56 @@ Drupal.edit.EntityModel = Backbone.Model.extend({
} }
}, },
/**
* Updates a Field and Entity model's "inTempStore" when appropriate.
*
* Helper function.
*
* @param Drupal.edit.EntityModel entityModel
* The model of the entity for which a field's state attribute has changed.
* @param Drupal.edit.FieldModel fieldModel
* The model of the field whose state attribute has changed.
*
* @see fieldStateChange()
*/
_updateInTempStoreAttributes: function (entityModel, fieldModel) {
var current = fieldModel.get('state');
var previous = fieldModel.previous('state');
var fieldsInTempStore = entityModel.get('fieldsInTempStore');
// If the fieldModel changed to the 'saved' state: remember that this
// field was saved to TempStore.
if (current === 'saved') {
// Mark the entity as saved in TempStore, so that we can pass the
// proper "reset TempStore" boolean value when communicating with the
// server.
entityModel.set('inTempStore', true);
// Mark the field as saved in TempStore, so that visual indicators
// signifying just that may be rendered.
fieldModel.set('inTempStore', true);
// Remember that this field is in TempStore, restore when rerendered.
fieldsInTempStore.push(fieldModel.get('fieldID'));
fieldsInTempStore = _.uniq(fieldsInTempStore);
entityModel.set('fieldsInTempStore', fieldsInTempStore);
}
// If the fieldModel changed to the 'candidate' state from the
// 'inactive' state, then this is a field for this entity that got
// rerendered. Restore its previous 'inTempStore' attribute value.
else if (current === 'candidate' && previous === 'inactive') {
fieldModel.set('inTempStore', _.intersection([fieldModel.get('fieldID')], fieldsInTempStore).length > 0);
}
},
/** /**
* Reacts to state changes in this entity's fields. * Reacts to state changes in this entity's fields.
* *
* @param Drupal.edit.FieldModel fieldModel * @param Drupal.edit.FieldModel fieldModel
* The model of the field whose state property changed. * The model of the field whose state attribute changed.
* @param String state * @param String state
* The state of the associated field. One of Drupal.edit.FieldModel.states. * The state of the associated field. One of Drupal.edit.FieldModel.states.
*/ */
fieldStateChange: function (fieldModel, state) { fieldStateChange: function (fieldModel, state) {
var entityModel = this; var entityModel = this;
var fieldState = state; var fieldState = state;
var fieldsInTempStore = this.get('fieldsInTempStore');
// Switch on the entityModel state. // Switch on the entityModel state.
// The EntityModel responds to FieldModel state changes as a function of its // The EntityModel responds to FieldModel state changes as a function of its
// state. For example, a field switching back to 'candidate' state when its // state. For example, a field switching back to 'candidate' state when its
...@@ -245,43 +276,22 @@ Drupal.edit.EntityModel = Backbone.Model.extend({ ...@@ -245,43 +276,22 @@ Drupal.edit.EntityModel = Backbone.Model.extend({
if (fieldState === 'changed') { if (fieldState === 'changed') {
entityModel.set('isDirty', true); entityModel.set('isDirty', true);
} }
// If the fieldModel changed to the 'saved' state: remember that this else {
// field was saved to TempStore. this._updateInTempStoreAttributes(entityModel, fieldModel);
else if (fieldState === 'saved') {
// Mark the entity as saved in TempStore, so that we can pass the
// proper "reset TempStore" boolean value when communicating with the
// server.
entityModel.set('inTempStore', true);
// Mark the field as saved in TempStore, so that visual indicators
// signifying just that may be rendered.
fieldModel.set('inTempStore', true);
// Remember that this field is in TempStore, restore when rerendered.
fieldsInTempStore.push(fieldModel.get('fieldID'));
fieldsInTempStore = _.uniq(fieldsInTempStore);
entityModel.set('fieldsInTempStore', fieldsInTempStore);
}
// If the fieldModel changed to the 'candidate' state from the
// 'inactive' state, then this is a field for this entity that got
// rerendered. Restore its previous 'inTempStore' attribute value.
else if (fieldState === 'candidate' && fieldModel.previous('state') === 'inactive') {
fieldModel.set('inTempStore', _.intersection([fieldModel.get('fieldID')], fieldsInTempStore).length > 0);
} }
break; break;
case 'committing': case 'committing':
// If the field save returned a validation error, set the state of the // If the field save returned a validation error, set the state of the
// entity back to opened. // entity back to 'opened'.
if (fieldState === 'invalid') { if (fieldState === 'invalid') {
// A state change in reaction to another state change must be deferred. // A state change in reaction to another state change must be deferred.
_.defer(function() { _.defer(function() {
entityModel.set('state', 'opened', { reason: 'invalid' }); entityModel.set('state', 'opened', { reason: 'invalid' });
}); });
} }
// If the fieldModel changed to the 'candidate' state from the else {
// 'inactive' state, then this is a field for this entity that got this._updateInTempStoreAttributes(entityModel, fieldModel);
// rerendered. Restore its previous 'inTempStore' attribute value.
else if (fieldState === 'candidate' && fieldModel.previous('state') === 'inactive') {
fieldModel.set('inTempStore', _.intersection([fieldModel.get('fieldID')], fieldsInTempStore).length > 0);
} }
// Attempt to save the entity. If the entity's fields are not yet all in // Attempt to save the entity. If the entity's fields are not yet all in
...@@ -503,6 +513,8 @@ Drupal.edit.EntityModel = Backbone.Model.extend({ ...@@ -503,6 +513,8 @@ Drupal.edit.EntityModel = Backbone.Model.extend({
destroy: function (options) { destroy: function (options) {
Backbone.Model.prototype.destroy.apply(this, options); Backbone.Model.prototype.destroy.apply(this, options);
this.off(null, null, this);
// Destroy all fields of this entity. // Destroy all fields of this entity.
this.get('fields').each(function (fieldModel) { this.get('fields').each(function (fieldModel) {
fieldModel.destroy(); fieldModel.destroy();
......
...@@ -82,7 +82,7 @@ Drupal.edit.FieldModel = Backbone.Model.extend({ ...@@ -82,7 +82,7 @@ Drupal.edit.FieldModel = Backbone.Model.extend({
if (this.get('state') !== 'inactive') { if (this.get('state') !== 'inactive') {
throw new Error("FieldModel cannot be destroyed if it is not inactive state."); throw new Error("FieldModel cannot be destroyed if it is not inactive state.");
} }
Backbone.Model.prototype.destroy.apply(this, options); Backbone.Model.prototype.destroy.call(this, options);
}, },
/** /**
...@@ -97,11 +97,6 @@ Drupal.edit.FieldModel = Backbone.Model.extend({ ...@@ -97,11 +97,6 @@ Drupal.edit.FieldModel = Backbone.Model.extend({
* {@inheritdoc} * {@inheritdoc}
*/ */
validate: function (attrs, options) { validate: function (attrs, options) {
// We only care about validating the 'state' attribute.
if (!_.has(attrs, 'state')) {
return;
}
var current = this.get('state'); var current = this.get('state');
var next = attrs.state; var next = attrs.state;
if (current !== next) { if (current !== next) {
......
...@@ -436,31 +436,48 @@ Drupal.edit.AppView = Backbone.View.extend({ ...@@ -436,31 +436,48 @@ Drupal.edit.AppView = Backbone.View.extend({
var $fieldWrapper = $(fieldModel.get('el')); var $fieldWrapper = $(fieldModel.get('el'));
var $context = $fieldWrapper.parent(); var $context = $fieldWrapper.parent();
var renderField = function () {
// Destroy the field model; this will cause all attached views to be
// destroyed too, and removal from all collections in which it exists.
fieldModel.destroy();
// Replace the old content with the new content.
$fieldWrapper.replaceWith(html);
// Attach behaviors again to the modified piece of HTML; this will
// create a new field model and call rerenderedFieldToCandidate() with
// it.
Drupal.attachBehaviors($context);
};
// When propagating the changes of another instance of this field, this // When propagating the changes of another instance of this field, this
// field is not being actively edited and hence no state changes are // field is not being actively edited and hence no state changes are
// necessary. So: only update the state of this field when the rerendering // necessary. So: only update the state of this field when the rerendering
// of this field happens not because of propagation, but because it is being // of this field happens not because of propagation, but because it is
// edited itself. // being edited itself.
if (!options.propagation) { if (!options.propagation) {
// First set the state to 'candidate', to allow all attached views to // Deferred because renderUpdatedField is reacting to a field model change
// clean up all their "active state"-related changes. // event, and we want to make sure that event fully propagates before
fieldModel.set('state', 'candidate'); // making another change to the same model.
_.defer(function () {
// First set the state to 'candidate', to allow all attached views to
// clean up all their "active state"-related changes.
fieldModel.set('state', 'candidate');
// Similarly, the above .set() call's change event must fully propagate
// before calling it again.
_.defer(function () {
// Set the field's state to 'inactive', to enable the updating of its
// DOM value.
fieldModel.set('state', 'inactive', { reason: 'rerender' });
// Set the field's state to 'inactive', to enable the updating of its DOM renderField();
// value. });
fieldModel.set('state', 'inactive', { reason: 'rerender' }); });
}
else {
renderField();
} }
// Destroy the field model; this will cause all attached views to be
// destroyed too, and removal from all collections in which it exists.
fieldModel.destroy();
// Replace the old content with the new content.
$fieldWrapper.replaceWith(html);
// Attach behaviors again to the modified piece of HTML; this will create
// a new field model and call rerenderedFieldToCandidate() with it.
Drupal.attachBehaviors($context);
}, },
/** /**
......
...@@ -35,7 +35,7 @@ Drupal.edit.ContextualLinkView = Backbone.View.extend({ ...@@ -35,7 +35,7 @@ Drupal.edit.ContextualLinkView = Backbone.View.extend({
*/ */
initialize: function (options) { initialize: function (options) {
// Insert the text of the quick edit toggle. // Insert the text of the quick edit toggle.
this.$el.find('a').text(this.options.strings.quickEdit); this.$el.find('a').text(options.strings.quickEdit);
// Initial render. // Initial render.
this.render(); this.render();
// Re-render whenever this entity's isActive attribute changes. // Re-render whenever this entity's isActive attribute changes.
......
...@@ -13,9 +13,9 @@ Drupal.edit.EntityToolbarView = Backbone.View.extend({ ...@@ -13,9 +13,9 @@ Drupal.edit.EntityToolbarView = Backbone.View.extend({
events: function () { events: function () {
var map = { var map = {
'click.edit button.action-save': 'onClickSave', 'click button.action-save': 'onClickSave',
'click.edit button.action-cancel': 'onClickCancel', 'click button.action-cancel': 'onClickCancel',
'mouseenter.edit': 'onMouseenter' 'mouseenter': 'onMouseenter'
}; };
return map; return map;
}, },
...@@ -117,7 +117,15 @@ Drupal.edit.EntityToolbarView = Backbone.View.extend({ ...@@ -117,7 +117,15 @@ Drupal.edit.EntityToolbarView = Backbone.View.extend({
* {@inheritdoc} * {@inheritdoc}
*/ */
remove: function () { remove: function () {
// Remove additional DOM elements controlled by this View.
this.$fence.remove(); this.$fence.remove();
// Stop listening to additional events.
this.appModel.off(null, null, this);
this.model.get('fields').off(null, null, this);
$(window).off('resize.edit scroll.edit');
$(document).off('drupalViewportOffsetChange.edit');
Backbone.View.prototype.remove.call(this); Backbone.View.prototype.remove.call(this);
}, },
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment