New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Popup("destroy") never ends #5244

Closed
DzenisevichK opened this Issue Nov 2, 2012 · 2 comments

Comments

Projects
None yet
2 participants
@DzenisevichK

DzenisevichK commented Nov 2, 2012

Test case:

http://jsbin.com/epowaq/7

Steps:

  • Click on "Open Popup"
  • Close popup - you will entered in "popup._destroy -> container.remove -> cleanData -> popup._destroy" loop that never ends.

Final results on different platforms:

  • ~1 min hung and then stack execution timeout on
    • (mobile Safari on iOS 4.3.3),
    • (mobile Safari on iOS 6),
  • ~3 sec hung on
    • (Chrome 22.0.1229.94 m on Windows 7 SP1 x64)
    • (IE9 on Windows 7 SP1 x64)
    • (Safari 5.1.7 on Windows 7 SP1 x64)

Related issue #5150

@gabrielschulhof

This comment has been minimized.

Show comment
Hide comment
@gabrielschulhof

gabrielschulhof Nov 2, 2012

Contributor

Good catch! If the payload element is not initially attached to the DOM, there's no place to put it back, so it stays inside the container. I shall have to detach() it and then remove the container.

Contributor

gabrielschulhof commented Nov 2, 2012

Good catch! If the payload element is not initially attached to the DOM, there's no place to put it back, so it stays inside the container. I shall have to detach() it and then remove the container.

@DzenisevichK

This comment has been minimized.

Show comment
Hide comment
@DzenisevichK

DzenisevichK commented Nov 2, 2012

@gabrielschulhof

Great! Thanks a lot!

gabrielschulhof added a commit that referenced this issue Nov 2, 2012

[popup] Avoid infinite recursion by detach()ing the payload from the …
…container before attempting to put it back to its original place in the DOM (which may not exist, if the popup was created based on a detached element) -- Fixes #5244

Conflicts:

	js/widgets/popup.js
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment