[librsvg: 15/20] PropertyBag: take in the element's namespace to assign it to attributes

From: Federico Mena Quintero <federico src gnome org>
To: commits-list gnome org
Cc:
Subject: [librsvg: 15/20] PropertyBag: take in the element's namespace to assign it to attributes
Date: Sat, 26 Oct 2019 23:36:09 +0000 (UTC)
commit 05581a73f2a6b4bef20a29fb047825f49c978410
Author: Federico Mena Quintero <federico gnome org>
Date:   Sat Oct 26 10:51:17 2019 -0500

    PropertyBag: take in the element's namespace to assign it to attributes
    
    Per the xmlns spec (https://www.w3.org/TR/xml-names11/ section 6.2),
    attributes without a namespace prefix get assigned the namespace of
    the element in which they appear.
    
    We don't do default namespaces for elements yet; that will come next.
    
    This fixes the tests that use <xi:include href="..."> and other
    attributes in xi:include.

 Cargo.lock                         | 14 ++++++-------
 rsvg_internals/src/property_bag.rs | 16 ++++++++++-----
 rsvg_internals/src/util.rs         | 24 -----------------------
 rsvg_internals/src/xml2_load.rs    | 40 ++++++++++++++++++++++++++++----------
 4 files changed, 48 insertions(+), 46 deletions(-)
---
diff --git a/Cargo.lock b/Cargo.lock
index 855d3ab8..f4f9fd38 100644
--- a/Cargo.lock
+++ b/Cargo.lock
@@ -350,7 +350,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index";
 
 [[package]]
 name = "float-cmp"
-version = "0.5.2"
+version = "0.5.3"
 source = "registry+https://github.com/rust-lang/crates.io-index";
 dependencies = [
  "num-traits 0.2.8 (registry+https://github.com/rust-lang/crates.io-index)",
@@ -411,7 +411,7 @@ dependencies = [
 
 [[package]]
 name = "getrandom"
-version = "0.1.12"
+version = "0.1.13"
 source = "registry+https://github.com/rust-lang/crates.io-index";
 dependencies = [
  "cfg-if 0.1.10 (registry+https://github.com/rust-lang/crates.io-index)",
@@ -910,7 +910,7 @@ name = "rand"
 version = "0.7.2"
 source = "registry+https://github.com/rust-lang/crates.io-index";
 dependencies = [
- "getrandom 0.1.12 (registry+https://github.com/rust-lang/crates.io-index)",
+ "getrandom 0.1.13 (registry+https://github.com/rust-lang/crates.io-index)",
  "libc 0.2.65 (registry+https://github.com/rust-lang/crates.io-index)",
  "rand_chacha 0.2.1 (registry+https://github.com/rust-lang/crates.io-index)",
  "rand_core 0.5.1 (registry+https://github.com/rust-lang/crates.io-index)",
@@ -954,7 +954,7 @@ name = "rand_core"
 version = "0.5.1"
 source = "registry+https://github.com/rust-lang/crates.io-index";
 dependencies = [
- "getrandom 0.1.12 (registry+https://github.com/rust-lang/crates.io-index)",
+ "getrandom 0.1.13 (registry+https://github.com/rust-lang/crates.io-index)",
 ]
 
 [[package]]
@@ -1113,7 +1113,7 @@ dependencies = [
  "data-url 0.1.0 (registry+https://github.com/rust-lang/crates.io-index)",
  "downcast-rs 1.1.0 (registry+https://github.com/rust-lang/crates.io-index)",
  "encoding 0.2.33 (registry+https://github.com/rust-lang/crates.io-index)",
- "float-cmp 0.5.2 (registry+https://github.com/rust-lang/crates.io-index)",
+ "float-cmp 0.5.3 (registry+https://github.com/rust-lang/crates.io-index)",
  "gdk-pixbuf 0.7.0 (registry+https://github.com/rust-lang/crates.io-index)",
  "gdk-pixbuf-sys 0.9.0 (registry+https://github.com/rust-lang/crates.io-index)",
  "gio 0.7.0 (registry+https://github.com/rust-lang/crates.io-index)",
@@ -1419,14 +1419,14 @@ source = "registry+https://github.com/rust-lang/crates.io-index";
 "checksum encoding-index-singlebyte 1.20141219.5 (registry+https://github.com/rust-lang/crates.io-index)" = 
"3351d5acffb224af9ca265f435b859c7c01537c0849754d3db3fdf2bfe2ae84a"
 "checksum encoding-index-tradchinese 1.20141219.5 (registry+https://github.com/rust-lang/crates.io-index)" = 
"fd0e20d5688ce3cab59eb3ef3a2083a5c77bf496cb798dc6fcdb75f323890c18"
 "checksum encoding_index_tests 0.1.4 (registry+https://github.com/rust-lang/crates.io-index)" = 
"a246d82be1c9d791c5dfde9a2bd045fc3cbba3fa2b11ad558f27d01712f00569"
-"checksum float-cmp 0.5.2 (registry+https://github.com/rust-lang/crates.io-index)" = 
"7ef4eee449a2818084dad09f4fcd6e6e8932c482d8d94298493226782bb45b5e"
+"checksum float-cmp 0.5.3 (registry+https://github.com/rust-lang/crates.io-index)" = 
"75224bec9bfe1a65e2d34132933f2de7fe79900c96a0174307554244ece8150e"
 "checksum fragile 0.3.0 (registry+https://github.com/rust-lang/crates.io-index)" = 
"05f8140122fa0d5dcb9fc8627cfce2b37cc1500f752636d46ea28bc26785c2f9"
 "checksum fuchsia-cprng 0.1.1 (registry+https://github.com/rust-lang/crates.io-index)" = 
"a06f77d526c1a601b7c4cdd98f54b5eaabffc14d5f2f0296febdc7f357c6d3ba"
 "checksum futf 0.1.4 (registry+https://github.com/rust-lang/crates.io-index)" = 
"7c9c1ce3fa9336301af935ab852c437817d14cd33690446569392e65170aac3b"
 "checksum gdk-pixbuf 0.7.0 (registry+https://github.com/rust-lang/crates.io-index)" = 
"9726408ee1bbada83094326a99b9c68fea275f9dbb515de242a69e72051f4fcc"
 "checksum gdk-pixbuf-sys 0.9.0 (registry+https://github.com/rust-lang/crates.io-index)" = 
"b1d6778abf5764b9080a9345a16c5d16289426a3b3edd808a29a9061d431c465"
 "checksum generic-array 0.12.3 (registry+https://github.com/rust-lang/crates.io-index)" = 
"c68f0274ae0e023facc3c97b2e00f076be70e254bc851d972503b328db79b2ec"
-"checksum getrandom 0.1.12 (registry+https://github.com/rust-lang/crates.io-index)" = 
"473a1265acc8ff1e808cd0a1af8cee3c2ee5200916058a2ca113c29f2d903571"
+"checksum getrandom 0.1.13 (registry+https://github.com/rust-lang/crates.io-index)" = 
"e7db7ca94ed4cd01190ceee0d8a8052f08a247aa1b469a7f68c6a3b71afcf407"
 "checksum gio 0.7.0 (registry+https://github.com/rust-lang/crates.io-index)" = 
"6261b5d34c30c2d59f879e643704cf54cb44731f3a2038000b68790c03e360e3"
 "checksum gio-sys 0.9.0 (registry+https://github.com/rust-lang/crates.io-index)" = 
"778b856a70a32e2cc5dd5cc7fa1b0c4b6df924fdf5c82984bc28f30565657cfe"
 "checksum glib 0.8.2 (registry+https://github.com/rust-lang/crates.io-index)" = 
"be27232841baa43e0fd5ae003f7941925735b2f733a336dc75f07b9eff415e7b"
diff --git a/rsvg_internals/src/property_bag.rs b/rsvg_internals/src/property_bag.rs
index 3cefc0c9..90503c32 100644
--- a/rsvg_internals/src/property_bag.rs
+++ b/rsvg_internals/src/property_bag.rs
@@ -4,9 +4,9 @@ use std::mem;
 use std::slice;
 use std::str;
 
-use markup5ever::QualName;
+use markup5ever::{LocalName, Namespace, Prefix, QualName};
 
-use crate::util::{make_qual_name, opt_utf8_cstr, utf8_cstr};
+use crate::util::{opt_utf8_cstr, utf8_cstr};
 
 pub struct PropertyBag<'a>(Vec<(QualName, &'a str)>);
 
@@ -32,6 +32,7 @@ impl<'a> PropertyBag<'a> {
     /// `attrs` array, as the property bag does not copy the strings - it directly stores pointers
     /// into that array's strings.
     pub unsafe fn new_from_xml2_attributes(
+        element_ns: &Namespace,
         n_attributes: usize,
         attrs: *const *const libc::c_char,
     ) -> PropertyBag<'a> {
@@ -53,7 +54,12 @@ impl<'a> PropertyBag<'a> {
                 let prefix = opt_utf8_cstr(prefix);
                 let uri = opt_utf8_cstr(uri);
                 let localname = utf8_cstr(localname);
-                let qual_name = make_qual_name(prefix, uri, localname);
+
+                let qual_name = QualName::new(
+                    prefix.map(Prefix::from),
+                    uri.map(Namespace::from).unwrap_or_else(|| element_ns.clone()),
+                    LocalName::from(localname)
+                );
 
                 if !value_start.is_null() && !value_end.is_null() {
                     assert!(value_end >= value_start);
@@ -103,7 +109,7 @@ mod tests {
 
     #[test]
     fn empty_property_bag() {
-        let map = unsafe { PropertyBag::new_from_xml2_attributes(0, ptr::null()) };
+        let map = unsafe { PropertyBag::new_from_xml2_attributes(&ns!(svg), 0, ptr::null()) };
         assert_eq!(map.len(), 0);
     }
 
@@ -139,7 +145,7 @@ mod tests {
             v.push(val_end); // value_end
         }
 
-        let pbag = unsafe { PropertyBag::new_from_xml2_attributes(3, v.as_ptr()) };
+        let pbag = unsafe { PropertyBag::new_from_xml2_attributes(&ns!(svg), 3, v.as_ptr()) };
 
         let mut had_rx: bool = false;
         let mut had_ry: bool = false;
diff --git a/rsvg_internals/src/util.rs b/rsvg_internals/src/util.rs
index 2116cb6a..e8efdd7a 100644
--- a/rsvg_internals/src/util.rs
+++ b/rsvg_internals/src/util.rs
@@ -4,8 +4,6 @@ use std::borrow::Cow;
 use std::ffi::CStr;
 use std::str;
 
-use markup5ever::{namespace_url, ns, LocalName, Namespace, Prefix, QualName};
-
 /// Converts a `char *` which is known to be valid UTF-8 into a `&str`
 ///
 /// The usual `from_glib_none(s)` allocates an owned String.  The
@@ -43,25 +41,3 @@ pub fn clamp<T: PartialOrd>(val: T, low: T, high: T) -> T {
         val
     }
 }
-
-pub fn make_qual_name(prefix: Option<&str>, uri: Option<&str>, localname: &str) -> QualName {
-    let ns = if let Some(uri) = uri {
-        Namespace::from(uri)
-    } else {
-        // FIXME: This assumes that unprefixed attribute names default to the svg namespace.
-        // I.e. <foo bar="baz"/> would yield an svg:bar attribute.
-        //
-        // I'm not sure if this is how things are supposed to work if there is
-        // a second namespace embedded in the middle of SVG markup:
-        //
-        // <svg xmlns="http://www.w3.org/2000/svg";>
-        //   <g>
-        //     <something xmlns="http://example.com/something";>
-        //       <somethingelse foo="blah"/>
-        //                      ^^^ should this be assumed something:foo?
-        ns!(svg)
-    };
-
-    QualName::new(prefix.map(Prefix::from), ns, LocalName::from(localname))
-}
-
diff --git a/rsvg_internals/src/xml2_load.rs b/rsvg_internals/src/xml2_load.rs
index 7359354b..62c1d612 100644
--- a/rsvg_internals/src/xml2_load.rs
+++ b/rsvg_internals/src/xml2_load.rs
@@ -12,10 +12,11 @@ use std::str;
 use std::sync::Once;
 
 use glib::translate::*;
+use markup5ever::{namespace_url, ns, LocalName, Namespace, Prefix, QualName};
 
 use crate::error::LoadingError;
 use crate::property_bag::PropertyBag;
-use crate::util::{cstr, make_qual_name, opt_utf8_cstr, utf8_cstr};
+use crate::util::{cstr, opt_utf8_cstr, utf8_cstr};
 use crate::xml::XmlState;
 use crate::xml2::*;
 
@@ -90,7 +91,9 @@ unsafe extern "C" fn rsvg_sax_serror_cb(user_data: *mut libc::c_void, error: xml
         column,
         cstr(error.message)
     );
-    xml2_parser.state.error(ParseFromStreamError::XmlParseError(full_error_message));
+    xml2_parser
+        .state
+        .error(ParseFromStreamError::XmlParseError(full_error_message));
 }
 
 fn free_xml_parser_and_doc(parser: xmlParserCtxtPtr) {
@@ -119,7 +122,10 @@ unsafe extern "C" fn sax_get_entity_cb(
     assert!(!name.is_null());
     let name = utf8_cstr(name);
 
-    xml2_parser.state.entity_lookup(name).unwrap_or(ptr::null_mut())
+    xml2_parser
+        .state
+        .entity_lookup(name)
+        .unwrap_or(ptr::null_mut())
 }
 
 unsafe extern "C" fn sax_entity_decl_cb(
@@ -172,6 +178,19 @@ unsafe extern "C" fn sax_unparsed_entity_decl_cb(
     );
 }
 
+fn make_qual_name(prefix: Option<&str>, uri: Option<&str>, localname: &str) -> QualName {
+    // FIXME: If the element doesn't have a namespace URI, we are falling back
+    // to the SVG namespace.  In reality we need to take namespace scoping into account,
+    // i.e. handle the "default namespace" active at that point in the XML stack.
+    let element_ns = uri.map(Namespace::from).unwrap_or_else(|| ns!(svg));
+
+    QualName::new(
+        prefix.map(Prefix::from),
+        element_ns,
+        LocalName::from(localname),
+    )
+}
+
 unsafe extern "C" fn sax_start_element_ns_cb(
     user_data: *mut libc::c_void,
     localname: *mut libc::c_char,
@@ -190,11 +209,15 @@ unsafe extern "C" fn sax_start_element_ns_cb(
     let prefix = opt_utf8_cstr(prefix);
     let uri = opt_utf8_cstr(uri);
     let localname = utf8_cstr(localname);
+
     let qual_name = make_qual_name(prefix, uri, localname);
 
     let nb_attributes = nb_attributes as usize;
-    let pbag =
-        PropertyBag::new_from_xml2_attributes(nb_attributes, attributes as *const *const _);
+    let pbag = PropertyBag::new_from_xml2_attributes(
+        &qual_name.ns,
+        nb_attributes,
+        attributes as *const *const _,
+    );
 
     if let Err(e) = xml2_parser.state.start_element(qual_name, &pbag) {
         let _: () = e; // guard in case we change the error type later
@@ -217,6 +240,7 @@ unsafe extern "C" fn sax_end_element_ns_cb(
     let prefix = opt_utf8_cstr(prefix);
     let uri = opt_utf8_cstr(uri);
     let localname = utf8_cstr(localname);
+
     let qual_name = make_qual_name(prefix, uri, localname);
 
     xml2_parser.state.end_element(qual_name);
@@ -250,11 +274,7 @@ unsafe extern "C" fn sax_processing_instruction_cb(
     assert!(!target.is_null());
     let target = utf8_cstr(target);
 
-    let data = if data.is_null() {
-        ""
-    } else {
-        utf8_cstr(data)
-    };
+    let data = if data.is_null() { "" } else { utf8_cstr(data) };
 
     xml2_parser.state.processing_instruction(target, data);
 }
[Date Prev][Date Next] [Thread Prev][Thread Next] [Thread Index] [Date Index] [Author Index]