Join GitHub today
GitHub is home to over 50 million developers working together to host and review code, manage projects, and build software together.
Sign upPass intended parent to create_element #312
Conversation
|
I sort of believe that the intended parent parameter shouldn't be optional, since the spec did not mention that it is optional. |
|
@KiChjang probably... In some uses of |
|
@bors-servo delegate=cynicaldevil @cynicaldevil Would you be interested in reviewing this? |
|
|
| let insertion_point = self.appropriate_place_for_insertion(None); | ||
| let (node1, node2) = match insertion_point { | ||
| LastChild(ref p) | | ||
| BeforeSibling(ref p) => (p.clone(), None), | ||
| TableFosterParenting { ref element, ref prev_element } => (element.clone(), Some(prev_element.clone())), | ||
| }; | ||
|
|
||
| // Step 7. | ||
| let qname = QualName::new(None, ns, name); | ||
| let elem = create_element(&mut self.sink, qname.clone(), attrs.clone(), Some(&node1)); |
This comment has been minimized.
This comment has been minimized.
cynicaldevil
Oct 3, 2017
Contributor
This might cause problems, because if the TableFosterParenting arm is matched in the match block, then either node1 or node2 can be the parent. This decision is taken on Servo's side, because we need to access the DOM to figure out which node is the parent.
This comment has been minimized.
This comment has been minimized.
cbrewster
Oct 4, 2017
Author
Member
Hmm true... Would elements be in the same document? Should I send both to create_element?
This comment has been minimized.
This comment has been minimized.
cynicaldevil
Oct 5, 2017
Contributor
These elements are from the stack of open elements, where it is possible that two elements may not have the same document associated with them. So you need to pass both elements.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
cbrewster
Jun 27, 2018
Author
Member
I believe so, its been a while since I worked on these changes. I'm a bit busy at the moment with school, but I'll get back to this in a couple weeks.
|
|
| @@ -1,7 +1,7 @@ | |||
| [package] | |||
|
|
|||
| name = "xml5ever" | |||
| version = "0.12.1" | |||
| version = "0.12.2" | |||
This comment has been minimized.
This comment has been minimized.
cbrewster
Jul 26, 2018
Author
Member
I'm not sure if this should be bumped as 0.12.2 or 0.13.0 here.
This comment has been minimized.
This comment has been minimized.
nox
Jul 26, 2018
Member
0.13, markup5ever is a public dependency and it got a breaking bump, so the breaking bump needs to be propagated as such.
This comment has been minimized.
This comment has been minimized.
|
|
|
I don't have the bandwidth to finish this at the moment. |
cbrewster commentedSep 17, 2017
This is needed to resolve servo/servo#18277