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

RFC: Fix duplicate root html #510

Closed
wants to merge 2 commits into from
Closed

Conversation

@jjjjw
Copy link
Contributor

jjjjw commented Jun 9, 2013

This should not be merged because it still has some fail.

In grappling with #343 I started to pull apart the document element and the document node. I got stuck because now we have two code paths that want to create a document node; once when we create the window and the document, and then again when we create all the nodes in the tree. I am currently wondering which is the better way, maybe neither, and how to make one path clearly responsible. Thinking to set the 'Document' on a hypothetical 'DocumentNode' and then create the document along with the tree.

@jdm
Copy link
Member

jdm commented Jun 11, 2013

I think this is complicating the issue far more than it needs to. Instead, I think we should check whether the default node we create has any children before returning a result, and return the node's firstChild if it exists (after detaching it from its parent). Otherwise, we can return the default node instead.

@metajack
Copy link
Contributor

metajack commented Jun 11, 2013

Also will need a rebase.

@jjjjw
Copy link
Contributor Author

jjjjw commented Jun 16, 2013

Thanks for the comments! I'm trying to find time to finish this.

@jjjjw jjjjw closed this Jun 23, 2013
glennw pushed a commit to glennw/servo that referenced this pull request Jan 16, 2017
Merged all the projects under the Cargo workspace

This will speed up the builds a little. Also added CI testing for the sample/replay binaries.

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/webrender/510)
<!-- Reviewable:end -->
@jdm jdm mentioned this pull request Nov 5, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants
You can’t perform that action at this time.