Don't return a string from TreeSink::attach_declarative_shadow_root

[simonwuelker]

The string is unused, so this is just a pointless allocation. This change returns a bool indicating the operations success instead.

Additionally, pass a reference to the elements attributes. The callee can decide whether they need ownership of the vector or not (servo doesn't).

[nicoburns]

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

[simonwuelker]

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.

[simonwuelker]

This now includes a commit to bump html5ever to 0.34. The version bump is necessary because this is a breaking change. There is a passing try run in servo/servo#37736. Can someone with release merge permissions merge this and release to crates.io?

[simonwuelker]

0.34 still needs to be published.

[mrobinson]

I tried publishing today, but I got this error. Sorry, I haven't had time to get to the bottom of it:

error[E0308]: mismatched types
    --> src/tree_builder/mod.rs:1497:63
     |
1497 |             .attach_declarative_shadow(shadow_host, template, &tag.attrs)
     |              -------------------------                        ^^^^^^^^^^ expected `Vec<Attribute>`, found `&Vec<Attribute>`
     |              |
     |              arguments to this method are incorrect
     |
     = note: expected struct `Vec<_>`
             found reference `&Vec<_>`
note: method defined here
    --> /Users/martin/.cargo/registry/src/index.crates.io-6f17d22bba15001f/markup5ever-0.16.2/interface/tree_builder.rs:253:8
     |
253  |     fn attach_declarative_shadow(
     |        ^^^^^^^^^^^^^^^^^^^^^^^^^
help: consider removing the borrow
     |
1497 -             .attach_declarative_shadow(shadow_host, template, &tag.attrs)
1497 +             .attach_declarative_shadow(shadow_host, template, tag.attrs)
     |

error[E0308]: mismatched types
    --> src/tree_builder/mod.rs:1496:9
     |
1495 |       ) -> bool {
     |            ---- expected `bool` because of return type
1496 | /         self.sink
1497 | |             .attach_declarative_shadow(shadow_host, template, &tag.attrs)
     | |_________________________________________________________________________^ expected `bool`, found `Result<(), String>`
     |
     = note: expected type `bool`
                found enum `Result<(), String>`

For more information about this error, try `rustc --explain E0308`.
error: could not compile `html5ever` (lib) due to 2 previous errors
error: failed to verify package tarball

[jdm]

This requires a major version update for markup5ever as well, since the public interface changed.

[simonwuelker]

Oops, my bad. I opened #636 to bump markup5ever as well.