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

This pull request solves #34 #33

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from

Conversation

peter-klaesson-stratsys

#34

…le one instance, i.e only one CKE instead of multiple
@badsyntax
Copy link
Owner

Am struggling a little bit to follow how this works. (Can you add some tests?) It looks like this only accommodates reverts to DOM trees within separate documents (eg docs within iframes). It would be super cool if we can revert changes to /any/ DOM tree (I guess similar to the approach in issue #27).

I want to be able to do this:

test('Reverting specific DOM trees', function() {

    var d1 = document.createElement('div');
    d1.innerHTML = 'test test';
    findAndReplaceDOMText(/test/ig, d1, 'div');

    var d2 = document.createElement('div');
    d2.innerHTML = 'test test test';
    findAndReplaceDOMText(/test/ig, d2, 'div');

    htmlEqual(d1.innerHTML, '<div>test</div> <div>test</div>');
    htmlEqual(d2.innerHTML, '<div>test</div> <div>test</div> <div>test</div>');

    findAndReplaceDOMText.revert(d1);
    findAndReplaceDOMText.revert(d2);

    htmlEqual(d1.innerHTML, 'test test');
    htmlEqual(d2.innerHTML, 'test test test');
});

@johan-ludvigsson-stratsys

Looks like we mixed up this pull-request with a different bug, issue #34

@badsyntax
Copy link
Owner

Thanks for the update, it makes more sense now! The merge request looks like a good solution, but I want to spend some time reviewing before merging. Ideally I would also like this change to happen in https://github.com/padolsey/findAndReplaceDOMText as I don't want to make custom changes to this function within this project. Please bear with me!

@ghost ghost assigned badsyntax Mar 13, 2013
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.

3 participants