Skip to content
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

unscrollbar was deleting text and doubling content #13

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

khujo
Copy link

@khujo khujo commented Sep 26, 2011

The unscrollbar function wasn't working correctly. It fetched everything except text nodes, but including children of children. So all text was missing while other elements where doubled.

…ing except text nodes, but including children of children. So all text was missing while other elements where doubled.
The jQuery documentation says:

"To avoid memory leaks, jQuery removes other constructs such as data and event handlers from the child elements before removing the elements themselves."
@wagich
Copy link

wagich commented Apr 2, 2012

Just got bitten by this too:
I think the cleanest approach is to just change line 497 to:
var holder = this.container.find('.scrollbar-pane').children().clone(true);
creating an in-memory copy before cleaning up the dom and then reinserting.

@khujo
Copy link
Author

khujo commented Apr 12, 2012

I don't have anything in mind, but I think .children() does not copy text nodes. That's why I use .content(). Let me if I miss something.

@wagich
Copy link

wagich commented Apr 12, 2012

You are right of course: you need to use contents() not children(), but I still think the approach is valid. Since I'm using the same lookup as you, I'd also need :first on my "scrollbar-pane"-selector giving me:
var holder = this.container.find('.scrollbar-pane:first').contents().clone(true);

… by wagich.

Revert "In case of nested scrollbar-panes the content of all scrollbar-panes got attached to the root pane."

This reverts commit d80ac1d.
@khujo
Copy link
Author

khujo commented Apr 12, 2012

You are right. Your approach should fix the issue and looks a lot cleaner. I pushed the changes to my repository.

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

Successfully merging this pull request may close these issues.

2 participants