Skip to content

Commit cbe87dc

Browse files
author
bors-servo
authored
Auto merge of #277 - moonlightdrive:innerhtml-escape, r=jdm
change type ChildrenOnly to ChildrenOnly(Option<QualName>) This is necessary so that these nodes exist on the stack when we serialize the children nodes and may need the parent for context. Addresses servo/servo#14975 --- Opening this PR for feedback/review ~~This does work and solves the above bug. I'm not sure if this is the best implementation, as there's a bit of duplicated code. I thought about adding a boolean flag to `start_elem` that would determine whether the writes should be performed. But then I thought maybe when not writing, it's best not to call a function returning `io::Result<()>`?~~ Also not 100% sure if the xml implementation is the right thing to do Anyway, please let me know of any improvements/changes to make. Thanks :) <!-- Reviewable:start --> --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/html5ever/277) <!-- Reviewable:end -->
2 parents bc4cd15 + 015cfb0 commit cbe87dc

File tree

5 files changed

+27
-31
lines changed

5 files changed

+27
-31
lines changed

html5ever/src/serialize/mod.rs

Lines changed: 12 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -15,11 +15,11 @@ use {LocalName, QualName};
1515

1616
pub fn serialize<Wr, T>(writer: Wr, node: &T, opts: SerializeOpts) -> io::Result<()>
1717
where Wr: Write, T: Serialize {
18-
let mut ser = HtmlSerializer::new(writer, opts);
18+
let mut ser = HtmlSerializer::new(writer, opts.clone());
1919
node.serialize(&mut ser, opts.traversal_scope)
2020
}
2121

22-
#[derive(Copy, Clone)]
22+
#[derive(Clone)]
2323
pub struct SerializeOpts {
2424
/// Is scripting enabled?
2525
pub scripting_enabled: bool,
@@ -39,7 +39,7 @@ impl Default for SerializeOpts {
3939
fn default() -> SerializeOpts {
4040
SerializeOpts {
4141
scripting_enabled: true,
42-
traversal_scope: TraversalScope::ChildrenOnly,
42+
traversal_scope: TraversalScope::ChildrenOnly(None),
4343
create_missing_parent: false,
4444
}
4545
}
@@ -72,10 +72,18 @@ fn tagname(name: &QualName) -> LocalName {
7272

7373
impl<Wr: Write> HtmlSerializer<Wr> {
7474
fn new(writer: Wr, opts: SerializeOpts) -> Self {
75+
let html_name = match opts.traversal_scope {
76+
TraversalScope::IncludeNode | TraversalScope::ChildrenOnly(None) => None,
77+
TraversalScope::ChildrenOnly(Some(ref n)) => Some(tagname(n))
78+
};
7579
HtmlSerializer {
7680
writer: writer,
7781
opts: opts,
78-
stack: vec![Default::default()],
82+
stack: vec!(ElemInfo {
83+
html_name: html_name,
84+
ignore_children: false,
85+
processed_first_child: false,
86+
}),
7987
}
8088
}
8189

@@ -190,18 +198,6 @@ impl<Wr: Write> Serializer for HtmlSerializer<Wr> {
190198
}
191199

192200
fn write_text(&mut self, text: &str) -> io::Result<()> {
193-
let prepend_lf = text.starts_with("\n") && {
194-
let parent = self.parent();
195-
!parent.processed_first_child && match parent.html_name {
196-
Some(local_name!("pre")) | Some(local_name!("textarea")) | Some(local_name!("listing")) => true,
197-
_ => false,
198-
}
199-
};
200-
201-
if prepend_lf {
202-
try!(self.writer.write_all(b"\n"));
203-
}
204-
205201
let escape = match self.parent().html_name {
206202
Some(local_name!("style")) | Some(local_name!("script")) | Some(local_name!("xmp"))
207203
| Some(local_name!("iframe")) | Some(local_name!("noembed")) | Some(local_name!("noframes"))

html5ever/tests/serializer.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -165,15 +165,15 @@ test!(noframes_literal, r#"<noframes>(x & 1) < 2; y > "foo" + 'bar'</noframes>"#
165165

166166
test!(pre_lf_0, "<pre>foo bar</pre>");
167167
test!(pre_lf_1, "<pre>\nfoo bar</pre>", "<pre>foo bar</pre>");
168-
test!(pre_lf_2, "<pre>\n\nfoo bar</pre>");
168+
test!(pre_lf_2, "<pre>\n\nfoo bar</pre>", "<pre>\nfoo bar</pre>");
169169

170170
test!(textarea_lf_0, "<textarea>foo bar</textarea>");
171171
test!(textarea_lf_1, "<textarea>\nfoo bar</textarea>", "<textarea>foo bar</textarea>");
172-
test!(textarea_lf_2, "<textarea>\n\nfoo bar</textarea>");
172+
test!(textarea_lf_2, "<textarea>\n\nfoo bar</textarea>", "<textarea>\nfoo bar</textarea>");
173173

174174
test!(listing_lf_0, "<listing>foo bar</listing>");
175175
test!(listing_lf_1, "<listing>\nfoo bar</listing>", "<listing>foo bar</listing>");
176-
test!(listing_lf_2, "<listing>\n\nfoo bar</listing>");
176+
test!(listing_lf_2, "<listing>\n\nfoo bar</listing>", "<listing>\nfoo bar</listing>");
177177

178178
test!(comment_1, r#"<p>hi <!--world--></p>"#);
179179
test!(comment_2, r#"<p>hi <!-- world--></p>"#);

markup5ever/rcdom.rs

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -338,7 +338,7 @@ impl Default for RcDom {
338338
impl Serialize for Handle {
339339
fn serialize<S>(&self, serializer: &mut S, traversal_scope: TraversalScope) -> io::Result<()>
340340
where S: Serializer {
341-
match (traversal_scope, &self.data) {
341+
match (&traversal_scope, &self.data) {
342342
(_, &NodeData::Element { ref name, ref attrs, .. }) => {
343343
if traversal_scope == IncludeNode {
344344
try!(serializer.start_elem(name.clone(),
@@ -355,28 +355,28 @@ impl Serialize for Handle {
355355
Ok(())
356356
}
357357

358-
(ChildrenOnly, &NodeData::Document) => {
358+
(&ChildrenOnly(_), &NodeData::Document) => {
359359
for handle in self.children.borrow().iter() {
360360
try!(handle.clone().serialize(serializer, IncludeNode));
361361
}
362362
Ok(())
363363
}
364364

365-
(ChildrenOnly, _) => Ok(()),
365+
(&ChildrenOnly(_), _) => Ok(()),
366366

367-
(IncludeNode, &NodeData::Doctype { ref name, .. }) => {
367+
(&IncludeNode, &NodeData::Doctype { ref name, .. }) => {
368368
serializer.write_doctype(&name)
369369
}
370-
(IncludeNode, &NodeData::Text { ref contents }) => {
370+
(&IncludeNode, &NodeData::Text { ref contents }) => {
371371
serializer.write_text(&contents.borrow())
372372
}
373-
(IncludeNode, &NodeData::Comment { ref contents }) => {
373+
(&IncludeNode, &NodeData::Comment { ref contents }) => {
374374
serializer.write_comment(&contents)
375375
}
376-
(IncludeNode, &NodeData::ProcessingInstruction { ref target, ref contents }) => {
376+
(&IncludeNode, &NodeData::ProcessingInstruction { ref target, ref contents }) => {
377377
serializer.write_processing_instruction(target, contents)
378378
},
379-
(IncludeNode, &NodeData::Document) => {
379+
(&IncludeNode, &NodeData::Document) => {
380380
panic!("Can't serialize Document node itself")
381381
}
382382
}

markup5ever/serialize.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,10 +11,10 @@ use QualName;
1111
use std::io;
1212

1313
//§ serializing-html-fragments
14-
#[derive(Copy, Clone, PartialEq)]
14+
#[derive(Clone, PartialEq)]
1515
pub enum TraversalScope {
1616
IncludeNode,
17-
ChildrenOnly
17+
ChildrenOnly(Option<QualName>)
1818
}
1919

2020
pub trait Serialize {

xml5ever/src/serialize/mod.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ pub use markup5ever::serialize::{Serialize, Serializer, TraversalScope, AttrRef}
1212
use std::io::{self, Write};
1313
use tree_builder::NamespaceMap;
1414

15-
#[derive(Copy, Clone)]
15+
#[derive(Clone)]
1616
/// Struct for setting serializer options.
1717
pub struct SerializeOpts {
1818
/// Serialize the root node? Default: ChildrenOnly
@@ -22,7 +22,7 @@ pub struct SerializeOpts {
2222
impl Default for SerializeOpts {
2323
fn default() -> SerializeOpts {
2424
SerializeOpts {
25-
traversal_scope: TraversalScope::ChildrenOnly
25+
traversal_scope: TraversalScope::ChildrenOnly(None)
2626
}
2727
}
2828
}

0 commit comments

Comments
 (0)