minidom: breaking: Allow getting Element attributes by namespace

- Move Element.attributes to `AttrMap`, slowly using rxml's features and
  unrolling our own.
- Add Element::attr_ns that requires the attribute namespace.
  Element:attr defaults to rxml::Namespace::none() but the interface
  changes nonetheless for a &NcNameStr. Similar changes on
  `ElementBuilder` methods.
- Remove iterator structs for attributes, return a ref on the AttrMap
  directly as we don't need to keep attributes' internals hidden
  anymore.
- Enable rxml's `macros` feature within tests to access the `xml_ncname`
  macro.

Signed-off-by: pep <pep@bouah.net>
This commit is contained in:
Maxime “pep” Buquet 2025-09-27 16:49:42 +02:00 committed by pep
commit 19865e5f1e
14 changed files with 272 additions and 236 deletions

View file

@ -193,7 +193,7 @@ mod tests {
let query = ServicesQuery { type_: None };
let elem = Element::from(query);
assert!(elem.is("services", ns::EXT_DISCO));
assert_eq!(elem.attrs().next(), None);
assert_eq!(elem.attrs().into_iter().next(), None);
assert_eq!(elem.nodes().next(), None);
}

View file

@ -4,6 +4,7 @@
// License, v. 2.0. If a copy of the MPL was not distributed with this
// file, You can obtain one at http://mozilla.org/MPL/2.0/.
use xso::exports::rxml;
use xso::{
error::{Error, FromElementError},
AsXml, FromXml,
@ -254,9 +255,9 @@ impl TryFrom<Element> for Transport {
impl From<Transport> for Element {
fn from(transport: Transport) -> Element {
Element::builder("transport", ns::JINGLE_S5B)
.attr("sid", transport.sid)
.attr("dstaddr", transport.dstaddr)
.attr("mode", transport.mode)
.attr(rxml::xml_ncname!("sid").into(), transport.sid)
.attr(rxml::xml_ncname!("dstaddr").into(), transport.dstaddr)
.attr(rxml::xml_ncname!("mode").into(), transport.mode)
.append_all(match transport.payload {
TransportPayload::Candidates(candidates) => candidates
.into_iter()
@ -264,7 +265,7 @@ impl From<Transport> for Element {
.collect::<Vec<_>>(),
TransportPayload::Activated(cid) => {
vec![Element::builder("activated", ns::JINGLE_S5B)
.attr("cid", cid)
.attr(rxml::xml_ncname!("cid").into(), cid)
.build()]
}
TransportPayload::CandidateError => {
@ -272,7 +273,7 @@ impl From<Transport> for Element {
}
TransportPayload::CandidateUsed(cid) => {
vec![Element::builder("candidate-used", ns::JINGLE_S5B)
.attr("cid", cid)
.attr(rxml::xml_ncname!("cid").into(), cid)
.build()]
}
TransportPayload::ProxyError => {

View file

@ -293,6 +293,7 @@ mod tests {
use super::*;
use jid::{BareJid, FullJid};
use xso::error::FromElementError;
use xso::exports::rxml;
#[cfg(target_pointer_width = "32")]
#[test]
@ -602,25 +603,31 @@ mod tests {
fn presence_with_to() {
let presence = Presence::new(Type::None);
let elem: Element = presence.into();
assert_eq!(elem.attr("to"), None);
assert_eq!(elem.attr(rxml::xml_ncname!("to")), None);
let presence = Presence::new(Type::None).with_to(Jid::new("localhost").unwrap());
let elem: Element = presence.into();
assert_eq!(elem.attr("to"), Some("localhost"));
assert_eq!(elem.attr(rxml::xml_ncname!("to")), Some("localhost"));
let presence = Presence::new(Type::None).with_to(BareJid::new("localhost").unwrap());
let elem: Element = presence.into();
assert_eq!(elem.attr("to"), Some("localhost"));
assert_eq!(elem.attr(rxml::xml_ncname!("to")), Some("localhost"));
let presence =
Presence::new(Type::None).with_to(Jid::new("test@localhost/coucou").unwrap());
let elem: Element = presence.into();
assert_eq!(elem.attr("to"), Some("test@localhost/coucou"));
assert_eq!(
elem.attr(rxml::xml_ncname!("to")),
Some("test@localhost/coucou")
);
let presence =
Presence::new(Type::None).with_to(FullJid::new("test@localhost/coucou").unwrap());
let elem: Element = presence.into();
assert_eq!(elem.attr("to"), Some("test@localhost/coucou"));
assert_eq!(
elem.attr(rxml::xml_ncname!("to")),
Some("test@localhost/coucou")
);
}
#[test]

View file

@ -35,6 +35,7 @@ mod tests {
use crate::ns;
use minidom::Element;
use xso::error::{Error, FromElementError};
use xso::exports::rxml;
#[cfg(target_pointer_width = "32")]
#[test]
@ -80,13 +81,13 @@ mod tests {
let receipt = Request;
let elem: Element = receipt.into();
assert!(elem.is("request", ns::RECEIPTS));
assert_eq!(elem.attrs().count(), 0);
assert_eq!(elem.attrs().into_iter().count(), 0);
let receipt = Received {
id: String::from("coucou"),
};
let elem: Element = receipt.into();
assert!(elem.is("received", ns::RECEIPTS));
assert_eq!(elem.attr("id"), Some("coucou"));
assert_eq!(elem.attr(rxml::xml_ncname!("id")), Some("coucou"));
}
}

View file

@ -46,6 +46,7 @@ mod helpers {
use self::helpers::{parse_str, roundtrip_full};
use xso::exports::rxml;
use xso::{AsXml, FromXml, PrintRawXml};
// these are adverserial local names in order to trigger any issues with
@ -1790,9 +1791,9 @@ fn element_catch_one_and_many_parse_in_order() {
"<parent xmlns='urn:example:ns1'><child num='0'/><child num='1'/></parent>",
) {
Ok(ElementCatchOneAndMany { child, children }) => {
assert_eq!(child.attr("num"), Some("0"));
assert_eq!(child.attr(rxml::xml_ncname!("num")), Some("0"));
assert_eq!(children.len(), 1);
assert_eq!(children[0].attr("num"), Some("1"));
assert_eq!(children[0].attr(rxml::xml_ncname!("num")), Some("1"));
}
other => panic!("unexpected result: {:?}", other),
}

View file

@ -15,13 +15,13 @@ macro_rules! get_attr {
)
};
($elem:ident, $attr:tt, Option, $value:ident, $func:expr) => {
match $elem.attr($attr) {
match $elem.attr(::xso::exports::rxml::xml_ncname!($attr).into()) {
Some($value) => Some($func),
None => None,
}
};
($elem:ident, $attr:tt, Required, $value:ident, $func:expr) => {
match $elem.attr($attr) {
match $elem.attr(::xso::exports::rxml::xml_ncname!($attr).into()) {
Some($value) => $func,
None => {
return Err(xso::error::Error::Other(
@ -32,7 +32,7 @@ macro_rules! get_attr {
}
};
($elem:ident, $attr:tt, Default, $value:ident, $func:expr) => {
match $elem.attr($attr) {
match $elem.attr(::xso::exports::rxml::xml_ncname!($attr).into()) {
Some($value) => $func,
None => ::core::default::Default::default(),
}
@ -277,7 +277,7 @@ macro_rules! generate_attribute_enum {
impl From<$elem> for minidom::Element {
fn from(elem: $elem) -> minidom::Element {
minidom::Element::builder($name, crate::ns::$ns)
.attr($attr, match elem {
.attr(::xso::exports::rxml::xml_ncname!($attr).into(), match elem {
$($elem::$enum => $enum_name,)+
})
.build()
@ -343,9 +343,9 @@ macro_rules! check_no_attributes {
macro_rules! check_no_unknown_attributes {
($elem:ident, $name:tt, [$($attr:tt),*]) => (
#[cfg(not(feature = "disable-validation"))]
for (_attr, _) in $elem.attrs() {
for ((ns, attr), _) in $elem.attrs() {
$(
if _attr == $attr {
if *ns == ::xso::exports::rxml::Namespace::NONE && attr == $attr {
continue;
}
)*

View file

@ -8,7 +8,11 @@ use crate::message::MessagePayload;
use crate::ns;
use alloc::collections::BTreeMap;
use minidom::{Element, Node};
use xso::error::{Error, FromElementError};
use xso::exports::rxml;
use xso::{
error::{Error, FromElementError},
exports::rxml::Namespace,
};
// TODO: Use a proper lang type.
type Lang = String;
@ -72,7 +76,10 @@ impl TryFrom<Element> for XhtmlIm {
for child in elem.children() {
if child.is("body", ns::XHTML) {
let child = child.clone();
let lang = child.attr("xml:lang").unwrap_or("").to_string();
let lang = child
.attr_ns(rxml::Namespace::xml(), rxml::xml_ncname!("lang").into())
.unwrap_or("")
.to_string();
let body = Body::try_from(child)?;
match bodies.insert(lang, body) {
None => (),
@ -161,8 +168,13 @@ impl TryFrom<Element> for Body {
}
Ok(Body {
style: parse_css(elem.attr("style")),
xml_lang: elem.attr("xml:lang").map(|xml_lang| xml_lang.to_string()),
style: parse_css(elem.attr(rxml::xml_ncname!("style"))),
xml_lang: elem
.attr_ns(
&Into::<Namespace>::into(String::from("xml")),
rxml::xml_ncname!("lang").into(),
)
.map(|xml_lang| xml_lang.to_string()),
children,
})
}
@ -171,8 +183,15 @@ impl TryFrom<Element> for Body {
impl From<Body> for Element {
fn from(body: Body) -> Element {
Element::builder("body", ns::XHTML)
.attr("style", get_style_string(body.style))
.attr("xml:lang", body.xml_lang)
.attr(
rxml::xml_ncname!("style").into(),
get_style_string(body.style),
)
.attr_ns(
Into::<Namespace>::into(String::from("xml")),
rxml::xml_ncname!("lang").into(),
body.xml_lang,
)
.append_all(children_to_nodes(body.children))
.build()
}
@ -309,44 +328,52 @@ impl TryFrom<Element> for Tag {
Ok(match elem.name() {
"a" => Tag::A {
href: elem.attr("href").map(|href| href.to_string()),
style: parse_css(elem.attr("style")),
type_: elem.attr("type").map(|type_| type_.to_string()),
href: elem
.attr(rxml::xml_ncname!("href"))
.map(|href| href.to_string()),
style: parse_css(elem.attr(rxml::xml_ncname!("style"))),
type_: elem
.attr(rxml::xml_ncname!("type"))
.map(|type_| type_.to_string()),
children,
},
"blockquote" => Tag::Blockquote {
style: parse_css(elem.attr("style")),
style: parse_css(elem.attr(rxml::xml_ncname!("style"))),
children,
},
"br" => Tag::Br,
"cite" => Tag::Cite {
style: parse_css(elem.attr("style")),
style: parse_css(elem.attr(rxml::xml_ncname!("style"))),
children,
},
"em" => Tag::Em { children },
"img" => Tag::Img {
src: elem.attr("src").map(|src| src.to_string()),
alt: elem.attr("alt").map(|alt| alt.to_string()),
src: elem
.attr(rxml::xml_ncname!("src"))
.map(|src| src.to_string()),
alt: elem
.attr(rxml::xml_ncname!("alt"))
.map(|alt| alt.to_string()),
},
"li" => Tag::Li {
style: parse_css(elem.attr("style")),
style: parse_css(elem.attr(rxml::xml_ncname!("style"))),
children,
},
"ol" => Tag::Ol {
style: parse_css(elem.attr("style")),
style: parse_css(elem.attr(rxml::xml_ncname!("style"))),
children,
},
"p" => Tag::P {
style: parse_css(elem.attr("style")),
style: parse_css(elem.attr(rxml::xml_ncname!("style"))),
children,
},
"span" => Tag::Span {
style: parse_css(elem.attr("style")),
style: parse_css(elem.attr(rxml::xml_ncname!("style"))),
children,
},
"strong" => Tag::Strong { children },
"ul" => Tag::Ul {
style: parse_css(elem.attr("style")),
style: parse_css(elem.attr(rxml::xml_ncname!("style"))),
children,
},
_ => Tag::Unknown(children),
@ -367,13 +394,13 @@ impl From<Tag> for Element {
{
let mut attrs = vec![];
if let Some(href) = href {
attrs.push(("href", href));
attrs.push((rxml::xml_ncname!("href"), href));
}
if let Some(style) = get_style_string(style) {
attrs.push(("style", style));
attrs.push((rxml::xml_ncname!("style"), style));
}
if let Some(type_) = type_ {
attrs.push(("type", type_));
attrs.push((rxml::xml_ncname!("type"), type_));
}
attrs
},
@ -382,7 +409,7 @@ impl From<Tag> for Element {
Tag::Blockquote { style, children } => (
"blockquote",
match get_style_string(style) {
Some(style) => vec![("style", style)],
Some(style) => vec![(rxml::xml_ncname!("style"), style)],
None => vec![],
},
children,
@ -391,7 +418,7 @@ impl From<Tag> for Element {
Tag::Cite { style, children } => (
"cite",
match get_style_string(style) {
Some(style) => vec![("style", style)],
Some(style) => vec![(rxml::xml_ncname!("style"), style)],
None => vec![],
},
children,
@ -400,17 +427,17 @@ impl From<Tag> for Element {
Tag::Img { src, alt } => {
let mut attrs = vec![];
if let Some(src) = src {
attrs.push(("src", src));
attrs.push((rxml::xml_ncname!("src"), src));
}
if let Some(alt) = alt {
attrs.push(("alt", alt));
attrs.push((rxml::xml_ncname!("alt"), alt));
}
("img", attrs, vec![])
}
Tag::Li { style, children } => (
"li",
match get_style_string(style) {
Some(style) => vec![("style", style)],
Some(style) => vec![(rxml::xml_ncname!("style"), style)],
None => vec![],
},
children,
@ -418,7 +445,7 @@ impl From<Tag> for Element {
Tag::Ol { style, children } => (
"ol",
match get_style_string(style) {
Some(style) => vec![("style", style)],
Some(style) => vec![(rxml::xml_ncname!("style"), style)],
None => vec![],
},
children,
@ -426,7 +453,7 @@ impl From<Tag> for Element {
Tag::P { style, children } => (
"p",
match get_style_string(style) {
Some(style) => vec![("style", style)],
Some(style) => vec![(rxml::xml_ncname!("style"), style)],
None => vec![],
},
children,
@ -434,7 +461,7 @@ impl From<Tag> for Element {
Tag::Span { style, children } => (
"span",
match get_style_string(style) {
Some(style) => vec![("style", style)],
Some(style) => vec![(rxml::xml_ncname!("style"), style)],
None => vec![],
},
children,
@ -443,7 +470,7 @@ impl From<Tag> for Element {
Tag::Ul { style, children } => (
"ul",
match get_style_string(style) {
Some(style) => vec![("style", style)],
Some(style) => vec![(rxml::xml_ncname!("style"), style)],
None => vec![],
},
children,
@ -454,7 +481,7 @@ impl From<Tag> for Element {
};
let mut builder = Element::builder(name, ns::XHTML).append_all(children_to_nodes(children));
for (key, value) in attrs {
builder = builder.attr(key, value);
builder = builder.attr(key.into(), value);
}
builder.build()
}