Skip to content

/ basicHTML

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

Jump to bottom

Fix #54 - implements CHildNodes in Node #55

Merged
merged 1 commit into from Nov 22, 2020
Merged

Fix #54 - implements CHildNodes in Node #55

merged 1 commit into from Nov 22, 2020

Conversation

@WebReflection
Copy link
Owner

@WebReflection WebReflection commented Nov 22, 2020

No description provided.

@WebReflection
Fix #54 - implements CHildNodes in Node
2562bf2
@WebReflection WebReflection merged commit 1661183 into master Nov 22, 2020
@coreyfarrell

This comment has been minimized.

Sign in to view
Copy link
Contributor

@coreyfarrell coreyfarrell commented on src/Node.js in 2562bf2 Nov 22, 2020

I'm confused, does this actually belong here? Adding it to CharacterData and DocumentType, keeping it on Element would make sense to me. Implementing at Node causes Document and Attr classes to have ChildNode functions where I don't think they should?

My issue is fixed by this PR (thanks!) but I wanted to mention that it appears to implement ChildNode more widely than the specs and browsers.

This comment has been minimized.

Sign in to view
Copy link
Owner Author

@WebReflection WebReflection replied Nov 22, 2020

P.S. you have a point thought, this could land as CharacterData only thing, so I might change it right away.

This comment has been minimized.

Sign in to view
Copy link
Owner Author

@WebReflection WebReflection replied Nov 22, 2020

OK, CharacterData only extended, beside Element, so this should be OK now?

This comment has been minimized.

Sign in to view
Copy link
Contributor

@coreyfarrell coreyfarrell replied Nov 23, 2020

Looks great, thanks for the fix. I would never call document.remove() unless a bug caused the document reference to get passed someplace it doesn't belong. In that case it's nice that basicHTML will throw an error like the browser would.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants
@WebReflection @coreyfarrell
You can’t perform that action at this time.