[glom] Document: Save static image data as a child text node, not an attribute.

[Murray Cumming <murrayc src gnome org>]

commit 030bec4a9612acda88ff72efb3dfc74b4431db5d
Author: Murray Cumming <murrayc murrayc com>
Date:   Mon Nov 12 22:53:08 2012 +0100

    Document: Save static image data as a child text node, not an attribute.
    
            * glom/libglom/document/document.cc: load_after_layout_group(),
    save_before_layout_group(): Save LayoutItem_Image data as
    a child text node of a <value> node, instead of as an attribute,
    because this is generally better for large chunks of arbitrary text,
    and because this lets us specify the format using the existing code.
            * glom/glom_document.dtd: Update appropriately.

 ChangeLog                         |   11 +++++++++++
 glom/glom_document.dtd            |   11 ++++++++---
 glom/libglom/document/document.cc |   22 ++++++++++++++++++----
 glom/libglom/xml_utils.cc         |    1 +
 4 files changed, 38 insertions(+), 7 deletions(-)
---
diff --git a/ChangeLog b/ChangeLog
index 7635500..ead126a 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,16 @@
 2012-11-12  Murray Cumming  <murrayc murrayc com>
 
+        Document: Save static image data as a child text node, not an attribute.
+
+        * glom/libglom/document/document.cc: load_after_layout_group(),
+	save_before_layout_group(): Save LayoutItem_Image data as 
+	a child text node of a <value> node, instead of as an attribute,
+	because this is generally better for large chunks of arbitrary text,
+	and because this lets us specify the format using the existing code.
+        * glom/glom_document.dtd: Update appropriately.
+
+2012-11-12  Murray Cumming  <murrayc murrayc com>
+
         Export to po files: Avoid duplicates.
 
         * glom/libglom/document/document.cc: Use a std::find_if()
diff --git a/glom/glom_document.dtd b/glom/glom_document.dtd
index 7963026..42c6cf5 100644
--- a/glom/glom_document.dtd
+++ b/glom/glom_document.dtd
@@ -108,9 +108,12 @@ TODO: Should we specify the presence of child text nodes in the ELEMENT somehow?
 
     <!ELEMENT example_row (value*)>
 
+<!-- This node contains a value in its child text node.
+     column attribute: Optionally, what column number in a table this value is for.
+     format: Optionally, "base64" for base64-encoded image data. Otherwise, the deprecated GDA image format. -->
     <!ELEMENT value (#PCDATA)>
     <!ATTLIST value
-        column CDATA #REQUIRED
+        column CDATA #IMPLIED
         format CDATA #IMPLIED>
 
 <!-- relationships describe connections between databases. -->
@@ -261,8 +264,10 @@ TODO: Should we specify the presence of child text nodes in the ELEMENT somehow?
         title CDATA #REQUIRED>
 
 <!-- data_layout_image displays a static image (an image that is not from the database).
-     The text attribute is the base64-encoded image data (if format="base64". Otherwise it is the deprecated GDA image text format.) -->
-    <!ELEMENT data_layout_image (formatting?, position?)>
+     text attribute: Deprecated. This was the gda-formatted image data.
+     format attribute: Always "base64", unless the deprecated text attribute is used.
+     The child value node contains the image data.-->
+    <!ELEMENT data_layout_image (value?, formatting?, position?)>
     <!ATTLIST data_layout_image
         text CDATA #IMPLIED
         title CDATA #IMPLIED
diff --git a/glom/libglom/document/document.cc b/glom/libglom/document/document.cc
index 0eabc08..2718d26 100644
--- a/glom/libglom/document/document.cc
+++ b/glom/libglom/document/document.cc
@@ -102,7 +102,7 @@ static const char GLOM_NODE_DATA_LAYOUT_BUTTON[] = "data_layout_button";
 static const char GLOM_NODE_DATA_LAYOUT_TEXTOBJECT[] = "data_layout_text";
 static const char GLOM_NODE_DATA_LAYOUT_TEXTOBJECT_TEXT[] = "text";
 static const char GLOM_NODE_DATA_LAYOUT_IMAGEOBJECT[] = "data_layout_image";
-static const char GLOM_ATTRIBUTE_DATA_LAYOUT_IMAGEOBJECT_IMAGE[] = "text"; //TODO: Deprecate this and replace it with "image"
+static const char GLOM_ATTRIBUTE_DATA_LAYOUT_IMAGEOBJECT_IMAGE[] = "text"; //Was gda-formatted image data. Deprecated in favour of a child text node containing base64-formatted image data.
 static const char GLOM_NODE_DATA_LAYOUT_LINE[] = "data_layout_line";
 static const char GLOM_ATTRIBUTE_DATA_LAYOUT_LINE_START_X[] = "start_x";
 static const char GLOM_ATTRIBUTE_DATA_LAYOUT_LINE_START_Y[] = "start_y";
@@ -2145,7 +2145,20 @@ void Document::load_after_layout_group(const xmlpp::Element* node, const Glib::u
         sharedptr<LayoutItem_Image> item = sharedptr<LayoutItem_Image>::create();
         load_after_translations(element, item);
 
-        item->set_image(XmlUtils::get_node_attribute_value_as_value(element, GLOM_ATTRIBUTE_DATA_LAYOUT_IMAGEOBJECT_IMAGE, Field::TYPE_IMAGE));
+        Gnome::Gda::Value value_image;
+        const xmlpp::Element* nodeValue = XmlUtils::get_node_child_named(element, GLOM_NODE_VALUE);
+        if(nodeValue)
+        {
+          value_image = XmlUtils::get_node_text_child_as_value(nodeValue, Field::TYPE_IMAGE);
+        }
+
+        if(value_image.is_null())
+        {
+          //Try the deprecated way:
+          value_image = XmlUtils::get_node_attribute_value_as_value(element, GLOM_ATTRIBUTE_DATA_LAYOUT_IMAGEOBJECT_IMAGE, Field::TYPE_IMAGE);
+        }
+
+        item->set_image(value_image);
 
         item_added = item;
       }
@@ -3398,7 +3411,8 @@ void Document::save_before_layout_group(xmlpp::Element* node, const sharedptr<co
                 nodeItem = child->add_child(GLOM_NODE_DATA_LAYOUT_IMAGEOBJECT);
                 save_before_translations(nodeItem, imageobject);
 
-                XmlUtils::set_node_attribute_value_as_value(nodeItem, GLOM_ATTRIBUTE_DATA_LAYOUT_IMAGEOBJECT_IMAGE, imageobject->get_image(), Field::TYPE_IMAGE);
+                xmlpp::Element* nodeValue = nodeItem->add_child(GLOM_NODE_VALUE);
+                XmlUtils::set_node_text_child_as_value(nodeValue, imageobject->get_image(), Field::TYPE_IMAGE);
               }
               else
               {
@@ -4601,7 +4615,7 @@ guint Document::get_latest_known_document_format_version()
   // Version 6: (Glom 1.16). Extra show_all option for choices that show related records. Extra related choice fields are now a layout group instead of just a second field name.
   // Version 7: (Glom 1.20). New print layout details. Related records: Number of rows can be specified. All colors can now be in CSS3 string format (via GdkRGBA)
   // Version 8: (Glom 1.22). The database_title attribute is replaced by the title attribute.
-  // Version 9: (Glom 1.24). <value> tags now have a format="base64" attribute by default. Having no format attribute is deprecated.
+  // Version 9: (Glom 1.24). <value> tags now have a format="base64" attribute by default. Having no format attribute is deprecated. data_layout_image nodes now have child <value> nodes instead of using the "text" attribute to store image data.
 
   return 9;
 }
diff --git a/glom/libglom/xml_utils.cc b/glom/libglom/xml_utils.cc
index 2a13ea8..e585d65 100644
--- a/glom/libglom/xml_utils.cc
+++ b/glom/libglom/xml_utils.cc
@@ -209,6 +209,7 @@ float get_node_attribute_value_as_float(const xmlpp::Element* node, const Glib::
   return result;
 }
 
+//TODO: Stop using this. It's a bad idea to put values in attributes, which cannot escape all characters.
 void set_node_attribute_value_as_value(xmlpp::Element* node, const Glib::ustring& strAttributeName, const Gnome::Gda::Value& value,  Field::glom_field_type field_type)
 {
   NumericFormat format_ignored; //Because we use ISO format.