Skip to content
This repository was archived by the owner on Apr 12, 2024. It is now read-only.

fix(ngMessages): prevent race condition with ngAnimate #12903

Merged
merged 1 commit into from
Sep 22, 2015

Conversation

petebacondarwin
Copy link
Contributor

If ngMessage tried to add a message back in that was about to be removed
after an animation, the NgMessageController got confused and tried to detach
the newly added message, when the pending node was destroyed.

This change applies a unique attachId to the message object and its DOM
node when it is attached. This is then checked when a DOM node is being
destroyed to prevent unwanted calls to detach.

Closes #12856

@ThomasBurleson
Copy link

@petebacondarwin - this fixes the issues of destroying/removing the appropriate node(s). Should we also think about reusing node elements that will be destroyed and re-added? Perhaps this is an optimization that is not worth the complexity?

@petebacondarwin
Copy link
Contributor Author

@ThomasBurleson:

So we are currently reusing the message controller objects but not the message nodes. This was the main cause for the problem since we would associate both the node that was being removed and the node that was being added with the same message controller object.

The trouble is that the old node has been already scheduled for removal ("leave") via ngAnimate by the time that we realise that it could be reused. So without some changes inside ngAnimate to allow such nodes to be "recovered", it would be rather tricky.

@topherfangio
Copy link

@petebacondarwin Just tested and this does appear to fix the issue in material. Thanks so much!

@ThomasBurleson
Copy link

@petebacondarwin - thx for the explanation.

@petebacondarwin
Copy link
Contributor Author

@matsko can you cast your eye over this fix before I merge, please?

If `ngMessage` tried to add a message back in that was about to be removed
after an animation, the NgMessageController got confused and tried to detach
the newly added message, when the pending node was destroyed.

This change applies a unique `attachId` to the message object and its DOM
node when it is attached. This is then checked when a DOM node is being
destroyed to prevent unwanted calls to `detach`.

Closes angular#12856
Closes angular#12903
@petebacondarwin petebacondarwin merged commit 8366622 into angular:master Sep 22, 2015
petebacondarwin added a commit that referenced this pull request Sep 22, 2015
If `ngMessage` tried to add a message back in that was about to be removed
after an animation, the NgMessageController got confused and tried to detach
the newly added message, when the pending node was destroyed.

This change applies a unique `attachId` to the message object and its DOM
node when it is attached. This is then checked when a DOM node is being
destroyed to prevent unwanted calls to `detach`.

Closes #12856
Closes #12903
@petebacondarwin
Copy link
Contributor Author

Cherry-picked to 1.4.x too

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants