mirrors/signal-desktop
2
0
Fork 0
Code Issues Pull requests Projects Releases Packages 2 Wiki Activity Actions

Fix: Ensure that attachments are always rendered

Browse source 
Because we only attach AttachmentViews to the DOM when they fire their
'update' event, we were subject to a race condition. If that event fired
after the final Message.render(), then it would be properly attached to
the final DOM node. If it fired early, it would end up missing from
the visible DOM entirely, attached to the old, discarded version of
the message.

This change updates our handling of a second call to loadAttachments().
Instead of bailing out if we've been called before, we attempt to
re-add our child AttachmentViews to the current DOM. But only if the
'update' event has been fired, and if their current parent node is not
what is in the DOM.

FREEBIE
This commit is contained in:
Scott Nonnenberg 2017-06-06 17:28:32 -07:00
parent da8d49b5ed
commit f38d8eb4ae
1 changed files with 27 additions and 9 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

36
js/views/message_view.js
View file

@ -260,12 +260,32 @@
}))();
this.$('.avatar').replaceWith(avatarView.render().$('.avatar'));
},
appendAttachmentView: function(view) {
// We check for a truthy 'updated' here to ensure that a race condition in a
// multi-fetch() scenario doesn't add an AttachmentView to the DOM before
// its 'update' event is triggered.
var parent = this.$('.attachments')[0];
if (view.updated && parent !== view.el.parentNode) {
if (view.el.parentNode) {
parent.removeChild(view.el);
}
this.trigger('beforeChangeHeight');
this.$('.attachments').append(view.el);
this.trigger('afterChangeHeight');
}
},
loadAttachments: function() {
this.loadedAttachments = this.loadedAttachments || [];
// Messages can go from no attachments to some attachments (via key approval),
// but they can't go from some attachments to a different set of them.
// If we're called a second time, render() has replaced the DOM out from under
// us with $el.html(). We'll need to reattach our AttachmentViews to the new
// parent DOM nodes if the 'update' event has already fired.
if (this.loadedAttachments.length) {
for (var i = 0, max = this.loadedAttachments.length; i < max; i += 1) {
var view = this.loadedAttachments[i];
this.appendAttachmentView(view);
}
return;
}
@ -274,15 +294,13 @@
model: attachment,
timestamp: this.model.get('sent_at')
});
this.listenTo(view, 'update', function() {
if (!view.el.parentNode) {
this.trigger('beforeChangeHeight');
this.$('.attachments').append(view.el);
this.trigger('afterChangeHeight');
}
});
view.render();
this.loadedAttachments.push(view);
this.listenTo(view, 'update', function() {
view.updated = true;
this.appendAttachmentView(view);
});
}.bind(this));
}
});