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

Fix issue 772; Use existing iframe DOM element instead of document.ge… #773

Merged

Conversation

martinbelanger
Copy link

I'm not able to test with your automated tests.
I tried with jasmine and with qunit.

qunit in Chrome work, but shadowRoot not seems to be supported with PhantomJS.
with jasmine, even with the master code, the tests passed.

@davidjbradshaw
Copy link
Owner

So are the tests in this PR doing anything?

@martinbelanger
Copy link
Author

I kept the tests in case of you was willing to trying to fix it.
But I think it's ok to not make any other tests, because your tests assure that all is working again.
I don't added a new feature but I make a bug fix when getElementById is impossible.

I'm actualy trying to test an invalid getElementById instead of trying to make shadowDom works.

@davidjbradshaw
Copy link
Owner

Ok can you take the tests out then if they are not doing anything, the change is small enough that I am happy if the current test pass with this ok

@martinbelanger
Copy link
Author

Ok can you take the tests out then if they are not doing anything, the change is small enough that I am happy if the current test pass with this ok

Done

@davidjbradshaw davidjbradshaw merged commit 21ab3f4 into davidjbradshaw:master Nov 9, 2019
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.

None yet

3 participants