Skip to content

Don't return a string from TreeSink::attach_declarative_shadow_root #633

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

Merged
merged 2 commits into from
Jun 27, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ Add html5ever as a dependency in your [`Cargo.toml`](https://crates.io/) file:

```toml
[dependencies]
html5ever = "0.32"
html5ever = "0.34"
```

You should also take a look at [`examples/html2html.rs`], [`examples/print-rcdom.rs`], and the [API documentation][].
Expand Down
2 changes: 1 addition & 1 deletion html5ever/Cargo.toml
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
[package]
name = "html5ever"
version = "0.33.0"
version = "0.34.0"
authors = [ "The html5ever Project Developers" ]
license = "MIT OR Apache-2.0"
repository = "https://github.com/servo/html5ever"
Expand Down
4 changes: 2 additions & 2 deletions html5ever/src/tree_builder/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1492,9 +1492,9 @@ where
tag: &Tag,
shadow_host: &Handle,
template: &Handle,
) -> Result<(), String> {
) -> bool {
self.sink
.attach_declarative_shadow(shadow_host, template, tag.attrs.clone())
.attach_declarative_shadow(shadow_host, template, &tag.attrs)
}

fn create_formatting_element_for(&self, tag: Tag) -> Handle {
Expand Down
3 changes: 2 additions & 1 deletion html5ever/src/tree_builder/rules.rs
Original file line number Diff line number Diff line change
Expand Up @@ -173,7 +173,8 @@ where

// Step 3 - 8.
// Attach a shadow root with declarative shadow host element, mode, clonable, serializable, delegatesFocus, and "named".
if self.attach_declarative_shadow(&tag, &shadow_host, &template).is_err() {
let succeeded = self.attach_declarative_shadow(&tag, &shadow_host, &template);
if !succeeded {
// Step 8.1.1. Insert an element at the adjusted insertion location with template.
// Pop the current template element created in step 2 first.
self.pop();
Expand Down
12 changes: 6 additions & 6 deletions markup5ever/interface/tree_builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -249,16 +249,16 @@ pub trait TreeSink {
true
}

/// Attach declarative shadow
/// Attempt to attach a declarative shadow root at the given location.
///
/// Returns a boolean indicating whether the operation succeeded or not.
fn attach_declarative_shadow(
&self,
_location: &Self::Handle,
_template: &Self::Handle,
_attrs: Vec<Attribute>,
) -> Result<(), String> {
Err(String::from(
"No implementation for attach_declarative_shadow",
))
Comment on lines -259 to -261
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about making this &'static str? Then we still get the error message without the allocation.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But the string is discarded anyways. html5ever doesn't care why attaching the shadow root failed - maybe the element already is a shadow host, maybe the element is not in the html namespace. This message is not logged anywhere, and I don't think it needs to be either.

_attrs: &[Attribute],
) -> bool {
false
}
}

Expand Down
2 changes: 1 addition & 1 deletion rcdom/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ path = "lib.rs"

[dependencies]
tendril = "0.4"
html5ever = { version = "0.33", path = "../html5ever" }
html5ever = { version = "0.34", path = "../html5ever" }
markup5ever = { version = "0.16", path = "../markup5ever" }
xml5ever = { version = "0.23", path = "../xml5ever" }

Expand Down