SVG import: a kept filter's feImage holds its data URL, not the key

The prepass writes a key in place of each <feImage> data URL before usvg
reads the SVG, and the filters kept as Effect > SVG Filters were read from
that edited text, so their XML held the key. The effect's check refused it
and the filter was dropped with a warning. The filter's XML now has each
keyed href put back as it was written.

crates/svg/src/import.rs

@@ -188,7 +188,7 @@ pub(crate) fn import_finding(
         symbol_suffixes: HashMap::new(),
         pattern_suffixes: HashMap::new(),
         blends: fx::blends(&xml, &scan.markers),
-        filter_sources: fx::sources(&xml, &scan.markers),
+        filter_sources: fx::sources(&xml, &scan.markers, |h| files.data_href(h)),
         files,
         non_scaling: found.non_scaling,
         data: found.data,

crates/svg/src/import/files.rs

@@ -102,6 +102,8 @@ struct DataUrls {
     numbers: HashMap<Arc<str>, usize>,
     /// Each href: a data URL, then what it decoded to, or a link.
     urls: Vec<DataUrl>,
+    /// Each href as it was written, for the filters kept as written ([`Files::data_href`]).
+    hrefs: Vec<Arc<str>>,
 }
 
 enum DataUrl {
@@ -322,6 +324,7 @@ impl Files {
                     let href: Arc<str> = Arc::from(a.value());
                     let n = data.urls.len();
                     data.numbers.insert(href.clone(), n);
+                    data.hrefs.push(href.clone());
                     data.urls.push(if url { DataUrl::Encoded(href) } else { DataUrl::Link(href) });
                     n
                 }
@@ -601,6 +604,13 @@ impl Files {
         (known && h.len() == DATA_URL_KEY.len() + n.to_string().len()).then_some(n)
     }
 
+    /// The href whose key is `h`, as it was written: a filter kept as an effect holds its
+    /// `<feImage>` hrefs as written, not their keys ([`super::fx::sources`]).
+    pub(in crate::import) fn data_href(&self, h: &str) -> Option<Arc<str>> {
+        let n = self.data_number(h)?;
+        self.data.lock().ok()?.hrefs.get(n).cloned()
+    }
+
     /// The image of data URL `n`: decoded the first time ([`Files::decode`]), then kept; the link
     /// when href `n` is one.
     fn data_url(&self, n: usize, opts: &usvg::Options) -> Result<Option<ImageKind>, Arc<str>> {

crates/svg/src/import/fx.rs

@@ -61,8 +61,10 @@ impl Source {
 
 /// Each `<filter>` element with an id, as written, by id, with the document's style sheets read
 /// once for all of them. A source is checked and read by usvg only when art uses it, once.
-/// `markers` is the text with its data URLs left out ([`super::depth::prescan`]).
-pub(super) fn sources(xml: &roxmltree::Document, markers: &str) -> HashMap<String, Source> {
+/// `markers` is the text with its data URLs left out ([`super::depth::prescan`]). `href` gives the
+/// href, as written, of each key the prepass wrote in place of an `<feImage>` href
+/// ([`super::files::Files::data_href`]): the source holds that href.
+pub(super) fn sources(xml: &roxmltree::Document, markers: &str, href: impl Fn(&str) -> Option<Arc<str>>) -> HashMap<String, Source> {
     if !markers.contains("<filter") && !markers.contains(":filter") {
         return HashMap::new();
     }
@@ -74,13 +76,40 @@ pub(super) fn sources(xml: &roxmltree::Document, markers: &str) -> HashMap<Strin
         .filter_map(|(order, n)| {
             let id = n.attribute("id")?.to_string();
             let primitives = n.children().filter(roxmltree::Node::is_element).map(|p| p.tag_name().name().to_string()).collect();
-            let xml = vectorcraft_effects::svg_filter::element_xml(n, text)?;
+            let xml = hrefs_as_written(n, vectorcraft_effects::svg_filter::element_xml(n, text)?, &href);
             let styled = vectorcraft_effects::svg_filter::depends_on_styles(n, &styles);
             Some((id, Source { xml, primitives, order, styled, ..Default::default() }))
         })
         .collect()
 }
 
+/// `xml`, the text of filter `n` ([`vectorcraft_effects::svg_filter::element_xml`]), with each
+/// `<feImage>` href that `href` gives a written href for put back as written, XML-escaped.
+fn hrefs_as_written(n: roxmltree::Node, mut xml: String, href: &impl Fn(&str) -> Option<Arc<str>>) -> String {
+    // `element_xml` adds namespace declarations to the start tag only, ahead of every child.
+    let (start, end) = (n.range().start, n.range().end);
+    let Some(shift) = xml.len().checked_sub(end - start) else { return xml };
+    let mut written: Vec<_> = n
+        .descendants()
+        .filter(|p| p.is_element() && p.tag_name().name() == "feImage")
+        .flat_map(|p| p.attributes())
+        .filter(|a| a.name() == "href" && matches!(a.namespace(), None | Some("http://www.w3.org/1999/xlink")))
+        .filter_map(|a| {
+            let r = a.range_value();
+            // A value written through an entity lies outside the element's text: it stays.
+            (r.start > start && r.end < end).then_some(())?;
+            Some((r.start - start + shift..r.end - start + shift, a.value(), href(a.value())?))
+        })
+        .collect();
+    written.sort_by_key(|(r, ..)| std::cmp::Reverse(r.start));
+    for (r, key, h) in written {
+        if xml.get(r.clone()) == Some(key) {
+            xml.replace_range(r, &crate::xml_escape(&h));
+        }
+    }
+    xml
+}
+
 /// The element name of primitive `k` (`feGaussianBlur`).
 fn element_name(k: &Kind) -> String {
     let debug = format!("{k:?}");