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

The contents of a closed popup should not appear in "find" or "copy all text" #5892

Closed
rbu opened this Issue Apr 17, 2013 · 2 comments

Comments

Projects
None yet
2 participants
@rbu

rbu commented Apr 17, 2013

We're opening popups programmatically, so it makes no sense to keep the dom nodes around after they are closed. In a long-running app, this creates large cruft when you popup tooltips or notifications. They should be removed from the dom if jQM added the dom node by itself.

Furthermore, if they are left around in the DOM, they should be display: none instead of just moved off-screen where they are found by the browser's "find" or "copy all text" features. This already happens correctly for pages when they deactivate.

@gabrielschulhof

This comment has been minimized.

Show comment
Hide comment
@gabrielschulhof

gabrielschulhof Apr 17, 2013

Contributor

http://jsbin.com/onibuc/421

$( "<div><p>This is a popup</p></div>" )
  .appendTo( $.mobile.activePage )
  .popup()
  .on( "popupafterclose", function() {
     $( this ).remove();
  })
  .popup( "open" );

Please reopen if this does not work for you!

Contributor

gabrielschulhof commented Apr 17, 2013

http://jsbin.com/onibuc/421

$( "<div><p>This is a popup</p></div>" )
  .appendTo( $.mobile.activePage )
  .popup()
  .on( "popupafterclose", function() {
     $( this ).remove();
  })
  .popup( "open" );

Please reopen if this does not work for you!

@gabrielschulhof

This comment has been minimized.

Show comment
Hide comment
@gabrielschulhof

gabrielschulhof Apr 17, 2013

Contributor

That's a good point about the hiding. I'll re-open and adjust the issue accordingly.

Contributor

gabrielschulhof commented Apr 17, 2013

That's a good point about the hiding. I'll re-open and adjust the issue accordingly.

gabrielschulhof added a commit that referenced this issue Apr 18, 2013

gabrielschulhof added a commit that referenced this issue Apr 19, 2013

gabrielschulhof added a commit that referenced this issue Apr 19, 2013

Revert "Tests: Unit: Popup: Make sure closed popup is not :visible an…
…d that open popup is :visible. Tests #5892."

This reverts commit b69b430.

gabrielschulhof added a commit that referenced this issue Apr 19, 2013

Popup: Set visibility: hidden instead of display: none to remove the …
…need for un-hiding before height calculation.

This uses the solution mentioned in #5912 instead of the original, display: none -based solution for #5892.

gabrielschulhof added a commit that referenced this issue May 7, 2013

gabrielschulhof added a commit that referenced this issue May 7, 2013

Popup: Set visibility: hidden instead of display: none to remove the …
…need for un-hiding before height calculation.

This uses the solution mentioned in #5912 instead of the original, display: none -based solution for #5892.
(cherry picked from commit 62bb9e5)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment