Skip to content

Close a void element that set_tag_name renames to a non-void name #337

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.

Sign up for GitHub

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

Open
robobun wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Close a void element that set_tag_name renames to a non-void name #337

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 23 additions & 0 deletions src/html/tag.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,29 @@ macro_rules! tag_is_one_of {
};
}

impl Tag {
/// Whether `name` is one of the [void elements] of the HTML namespace
/// (plus the obsolete ones the tree builder still treats as void).
///
/// [void elements]: https://html.spec.whatwg.org/multipage/syntax.html#void-elements
#[inline]
#[must_use]
pub(crate) fn is_void_html_element(name: &impl PartialEq<Self>) -> bool {
// NOTE: fast path for the most commonly used elements
if tag_is_one_of!(*name, [Div, A, Span, Li]) {
return false;
}

tag_is_one_of!(
*name,
[
Area, Base, Basefont, Bgsound, Br, Col, Embed, Hr, Img, Input, Keygen, Link, Meta,
Param, Source, Track, Wbr
]
)
}
}

#[cfg(test)]
mod tests {
use super::*;
Expand Down
187 changes: 172 additions & 15 deletions src/rewritable_units/element.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ use super::{
};
use crate::HandlerResult;
use crate::base::{BytesCow, SourceLocation};
use crate::html::{LocalNameHash, Namespace, Tag};
use crate::rewriter::{HandlerTypes, LocalHandlerTypes};
use encoding_rs::Encoding;
use std::any::Any;
Expand Down Expand Up @@ -128,18 +129,31 @@ impl<'rewriter, 'input_token, H: HandlerTypes> Element<'rewriter, 'input_token,

/// Sets the tag name of the element.
///
/// The new tag name must be in the same namespace, have the same content model, and be valid in its location.
/// The new tag name must be in the same namespace and be valid in its location.
/// Otherwise change of the tag name may cause the resulting document to be parsed in an unexpected way,
/// out of sync with this library.
///
/// If the element is a [void element] (`<img>`, `<br>`, ...) and the new name is not, an end tag
/// `</name>` is written right after the start tag, so that the renamed element stays empty instead of
/// swallowing the siblings that follow it. The element still [can't have content][Self::can_have_content]:
/// [`prepend`][Self::prepend], [`append`][Self::append] and [`set_inner_content`][Self::set_inner_content]
/// remain no-ops, and the written end tag doesn't run [end tag handlers][Self::end_tag_handlers].
///
/// [void element]: https://html.spec.whatwg.org/multipage/syntax.html#void-elements
#[inline]
pub fn set_tag_name(&mut self, name: &str) -> Result<(), TagNameError> {
let name = self.tag_name_bytes_from_str(name)?;
let name_bytes = self.tag_name_bytes_from_str(name)?;

if self.can_have_content {
self.modified_end_tag_name = Some((*name).into());
self.modified_end_tag_name = Some((*name_bytes).into());
} else if self.start_tag.namespace() == Namespace::Html {
// A self-closing tag in foreign content is closed by its `/>` whatever its name is,
// but an HTML void element is closed by its name alone.
let emit_end_tag = !Tag::is_void_html_element(&LocalNameHash::from(name));
self.start_tag.set_emit_end_tag(emit_end_tag);
}

self.start_tag.set_name_raw(name);
self.start_tag.set_name_raw(name_bytes);

Ok(())
}
Expand Down Expand Up @@ -905,8 +919,8 @@ mod tests {
);
assert_eq!(
out,
"<math><style><TROUBLE></style></math>
<textarea><!--</textarea><TROUBLE>--></textarea>
"<math><style><TROUBLE></TROUBLE></style></math>
<textarea><!--</textarea><TROUBLE></TROUBLE>--></textarea>
<div><style><img></style></div>"
);
}
Expand All @@ -922,8 +936,8 @@ mod tests {
);
assert_eq!(
out,
"<svg><p><style><!--</style><BINGO>--></style>
<math><p></p><style><!--</style><BINGO src onerror>--></style></math>"
"<svg><p><style><!--</style><BINGO></BINGO>--></style>
<math><p></p><style><!--</style><BINGO src onerror></BINGO>--></style></math>"
);
}

Expand All @@ -935,7 +949,10 @@ mod tests {
"img",
|el| el.set_tag_name("we-have-scripts").unwrap(),
);
assert_eq!(out, r#"<noscript><p alt="</noscript><we-have-scripts>">"#);
assert_eq!(
out,
r#"<noscript><p alt="</noscript><we-have-scripts></we-have-scripts>">"#
);
}

#[test]
Expand All @@ -946,7 +963,7 @@ mod tests {
"img,a",
|el| el.set_tag_name("HIT").unwrap(),
);
assert_eq!(out, r#"<svg></p><style><a id="</style><HIT>">"#);
assert_eq!(out, r#"<svg></p><style><a id="</style><HIT></HIT>">"#);
}

#[test]
Expand All @@ -961,7 +978,7 @@ mod tests {
);
assert_eq!(
out,
r#"<math><br><style><a id="</style><HIT>">
r#"<math><br><style><a id="</style><HIT></HIT>">
<math><font kolor/><style><HIT/></style><body><style><a/></style></math>
<math><font COLOR/><style><a/></style>"#
);
Expand All @@ -986,7 +1003,7 @@ mod tests {
let out = rewrite_element(br"<svG><sCript/><img></script></svg>", UTF_8, "img", |el| {
el.set_tag_name("HIT").unwrap();
});
assert_eq!(out, r"<svG><sCript/><HIT></script></svg>");
assert_eq!(out, r"<svG><sCript/><HIT></HIT></script></svg>");
}

#[test]
Expand All @@ -1000,7 +1017,7 @@ mod tests {
);
assert_eq!(
out,
r#"<math><annotation-xml encoding="nope/html"><style><XML></style></annotation-xml></math>
r#"<math><annotation-xml encoding="nope/html"><style><XML></XML></style></annotation-xml></math>
<math><annotation-xml encoding="text/HTML"><style><img></style></annotation-xml></math>"#
);
}
Expand Down Expand Up @@ -1075,7 +1092,7 @@ mod tests {
);
assert_eq!(
out,
r#"<math><p></p><style><!--</style><A src onerror>--></style></math>"#
r#"<math><p></p><style><!--</style><A src onerror></A>--></style></math>"#
);
}

Expand Down Expand Up @@ -1505,7 +1522,147 @@ mod tests {
el.set_tag_name("img-foo").unwrap();
});

assert_eq!(output, "<img-foo><!--after--><span>Hi</span></img>");
assert_eq!(
output,
"<img-foo></img-foo><!--after--><span>Hi</span></img>"
);
}

#[test]
fn void_element_renamed() {
macro_rules! test {
($html:expr, $handler:expr, $expected:expr) => {
for enc in [UTF_8, EUC_JP] {
assert_eq!(
rewrite_element(&$html.as_bytes(), enc, "#v", $handler),
$expected
);
}
};
}

// To a name that isn't void: closed on the spot, siblings stay siblings.
test!(
"<img id=v src=a.png><p>next</p>",
|el| {
assert!(!el.can_have_content());
el.set_tag_name("picture").unwrap();
assert!(!el.can_have_content());
},
"<picture id=v src=a.png></picture><p>next</p>"
);
test!(
"<br id=v><p>next</p></body>",
|el| el.set_tag_name("textarea").unwrap(),
"<textarea id=v></textarea><p>next</p></body>"
);
test!(
"<input id=v value=x><b>sib</b>",
|el| el.set_tag_name("My-Input").unwrap(),
"<My-Input id=v value=x></My-Input><b>sib</b>"
);

// The `/>` syntax means nothing on an HTML element, so it is dropped rather than
// serializing the misleading `<div/>`.
test!(
"<br id=v />x",
|el| {
assert!(el.is_self_closing());
el.set_tag_name("div").unwrap();
assert!(!el.is_self_closing());
},
"<div id=v></div>x"
);

// To another void name: still no end tag (`</br>` would even parse as `<br>`).
test!(
"<hr id=v>x",
|el| el.set_tag_name("br").unwrap(),
"<br id=v>x"
);
test!(
"<hr id=v>x",
|el| el.set_tag_name("IMG").unwrap(),
"<IMG id=v>x"
);

// The last name set wins.
test!(
"<hr id=v>x",
|el| {
el.set_tag_name("div").unwrap();
el.set_tag_name("wbr").unwrap();
},
"<wbr id=v>x"
);

// `before`/`after` content surrounds the whole (now two-tag) element whatever the
// call order; content insertion is still a no-op.
test!(
"<img id=v>x",
|el| {
el.before("[b1]", ContentType::Html);
el.after("[a1]", ContentType::Html);
el.set_tag_name("span").unwrap();
el.before("[b2]", ContentType::Html);
el.after("[a2]", ContentType::Html);
el.prepend("[p]", ContentType::Html);
el.append("[a]", ContentType::Html);
el.set_inner_content("[i]", ContentType::Html);
assert!(el.end_tag_handlers().is_none());
},
"[b1][b2]<span id=v></span>[a2][a1]x"
);

// Removed or replaced: nothing of the element is left, end tag included.
test!(
"<img id=v>x",
|el| {
el.set_tag_name("span").unwrap();
el.remove();
},
"x"
);
test!(
"<img id=v>x",
|el| {
el.set_tag_name("span").unwrap();
el.remove_and_keep_content();
},
"x"
);
test!(
"<img id=v>x",
|el| {
el.set_tag_name("span").unwrap();
el.replace("[r]", ContentType::Text);
el.after("[a]", ContentType::Html);
},
"[r][a]x"
);
}

#[test]
fn self_closing_foreign_element_renamed() {
// In foreign content `/>` closes the element whatever it is called, so a rename
// needs no end tag...
let output = rewrite_element(b"<svg><circle id=v /><g></g></svg>", UTF_8, "#v", |el| {
assert!(!el.can_have_content());
el.set_tag_name("path").unwrap();
});
assert_eq!(output, r#"<svg><path id=v /><g></g></svg>"#);

// ...and without `/>` it has an end tag of its own to rename.
let output = rewrite_element(
b"<svg><circle id=v></circle><g></g></svg>",
UTF_8,
"#v",
|el| {
assert!(el.can_have_content());
el.set_tag_name("path").unwrap();
},
);
assert_eq!(output, r#"<svg><path id=v></path><g></g></svg>"#);
}

#[test]
Expand Down
26 changes: 26 additions & 0 deletions src/rewritable_units/tokens/start_tag.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,10 @@ pub struct StartTag<'input_token> {
attributes: Attributes<'input_token>,
ns: Namespace,
self_closing: bool,
/// Serialize `</name>` right after this tag. Set when an HTML void
/// element is renamed to a name that is not void, so that nothing in the
/// input would otherwise close it.
emit_end_tag: bool,
raw: SpannedRawBytes<'input_token>,
pub(crate) mutations: Mutations,
}
Expand All @@ -38,11 +42,17 @@ impl<'input_token> StartTag<'input_token> {
attributes,
ns,
self_closing,
emit_end_tag: false,
raw,
mutations: Mutations::new(),
})
}

#[inline]
pub(crate) const fn namespace(&self) -> Namespace {
self.ns
}

#[inline(always)]
#[doc(hidden)]
pub const fn encoding(&self) -> &'static Encoding {
Expand Down Expand Up @@ -156,6 +166,16 @@ impl<'input_token> StartTag<'input_token> {
self.self_closing = has_slash;
}

/// If true, `</name>` is serialized right after the tag (and before any
/// [`Self::after`] content), and the `/>` syntax is dropped.
pub(crate) fn set_emit_end_tag(&mut self, emit_end_tag: bool) {
self.emit_end_tag = emit_end_tag;
if emit_end_tag {
self.self_closing = false;
}
self.raw.set_modified();
}

/// Inserts `content` before the start tag.
///
/// Consequent calls to the method append `content` to the previously inserted content.
Expand Down Expand Up @@ -261,6 +281,12 @@ impl<'input_token> StartTag<'input_token> {
} else {
output_handler(b">");
}

if self.emit_end_tag {
output_handler(b"</");
output_handler(&self.name);
output_handler(b">");
}
Ok(())
}

Expand Down
13 changes: 1 addition & 12 deletions src/selectors_vm/stack.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,18 +11,7 @@ use std::hash::BuildHasher;

#[inline]
fn is_void_element(local_name: &LocalName<'_>, enable_esi_tags: bool) -> bool {
// NOTE: fast path for the most commonly used elements
if tag_is_one_of!(*local_name, [Div, A, Span, Li]) {
return false;
}

if tag_is_one_of!(
*local_name,
[
Area, Base, Basefont, Bgsound, Br, Col, Embed, Hr, Img, Input, Keygen, Link, Meta,
Param, Source, Track, Wbr
]
) {
if Tag::is_void_html_element(local_name) {
return true;
}

Expand Down