After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 672716 - .gnumeric round-trip issues
.gnumeric round-trip issues
Status: RESOLVED FIXED
Product: Gnumeric
Classification: Applications
Component: import/export other
git master
Other All
: Normal normal
: ---
Assigned To: Morten Welinder
Jody Goldberg
Depends on:
Blocks:
 
 
Reported: 2012-03-23 19:54 UTC by Morten Welinder
Modified: 2012-04-02 16:54 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
conceptual patch (3.10 KB, patch)
2012-03-29 04:35 UTC, Andreas J. Guelzow
none Details | Review
proposed patch (6.05 KB, patch)
2012-03-31 07:11 UTC, Andreas J. Guelzow
none Details | Review
improved patch (5.07 KB, patch)
2012-03-31 17:35 UTC, Andreas J. Guelzow
committed Details | Review

Description Morten Welinder 2012-03-23 19:54:05 UTC
../src/ssconvert -T Gnumeric_XmlIO:sax ../samples/excel/objs.xls /tmp/obj.xml
../src/ssconvert -T Gnumeric_XmlIO:sax /tmp/obj.xml /tmp/obj2.xml
diff -u /tmp/obj{,2}.xml | less

1. We seem to have a minor type issue:
-      <meta:user-defined meta:name="gsf:document-parts"/>
-      <meta:user-defined meta:name="gsf:heading-pairs"/>
+      <meta:user-defined meta:name="gsf:document-parts" meta:value-type="string"></meta:user-defined>
+      <meta:user-defined meta:name="gsf:heading-pairs" meta:value-type="string"></meta:user-defined>


2. Something messes with "Inc" for scroll bars and spin buttons:
-        <gnm:SheetWidgetScrollbar Name="SCROLL" ... Inc="1" .../>
-        <gnm:SheetWidgetSpinbutton Name="SPIN" ... Inc="1" .../>
+        <gnm:SheetWidgetScrollbar Name="SCROLL" ... Inc="10" .../>
+        <gnm:SheetWidgetSpinbutton Name="SPIN" ... Inc="10" .../>


3. Something is up with images.
(too big)


4. Something changes the background colour in GogStyle land.
                   <property name="style" type="GogStyle">
                     <outline dash="solid" auto-dash="1" width="0" color="0:0:0:FF" auto-color="1"/>
                     <fill type="pattern" auto-type="1" is-auto="1" auto-fore="1">
-                      <pattern type="solid" fore="0:0:0:FF" back="9C:9C:FF:FF"/>
+                      <pattern type="solid" fore="0:0:0:FF" back="0:0:0:FF"/>
                     </fill>
                   </property>
Comment 1 Morten Welinder 2012-03-23 20:32:23 UTC
#2 fixed.
Comment 2 Morten Welinder 2012-03-24 00:58:50 UTC
Jean, something is wrong over in GOImage land.

We load an image with go_image_new_from_data, but go_pixbuf_save
(via go_image_save) is unable to save it.  (->data is null.)
Comment 3 Jean Bréfort 2012-03-24 06:25:28 UTC
Seen that. /tmp/obj.xml has the image saved the ancient way. These days, it should be saved at the GODoc level. This is on the top of my TODO list.
Comment 4 Morten Welinder 2012-03-24 12:25:17 UTC
/tmp/obj.xml uses the old way, but notice that somewhere between reading
the old format and writing the new one, the image is lost!

Actually, since the image shows when you do "gnumeric /tmp/obj.xml"
it must be a writing issue.
Comment 5 Morten Welinder 2012-03-25 19:10:14 UTC
The non-black colour is coming from map_area_series_solid_default
in gog-theme.c
Comment 6 Jean Bréfort 2012-03-26 08:28:46 UTC
#3 fixed. I can't reproduce #4 anymore, Morten, did you fix it?.
Comment 7 Morten Welinder 2012-03-26 13:38:34 UTC
> I can't reproduce #4 anymore, Morten, did you fix it?.

No, you did.  The xls importer changes now makes the background be
saved as 9C:9C:FF:FF in both cases.  For better or worse.

However the only reason #3 appears fixed is that the image now isn't saved
the first time around either:

        <gnm:SheetObjectImage Name="IMAGE" ObjectBound="F24:L47" ObjectOffset="0.208 0.422 0.598 0.211" Direction="17" Print="1" crop-top="0" crop-bottom="0" crop-left="0" crop-right="0">
          <Content image-type="png" name="Image"/>
        </gnm:SheetObjectImage>
...
  <GODoc>
    <GOImage name="Image" type="GOPixbuf" width="320" height="301" rowstride="1280"/>
  </GODoc>


Also, when loading objs.xml with gnumeric I get...
** (/home/welinder/gnome-src/gnumeric/src/.libs/gnumeric:6636): CRITICAL **: pixbuf_to_cairo: assertion `GO_IS_PIXBUF (pixbuf) && image->data && pixbuf->pixbuf' failed
Comment 8 Jean Bréfort 2012-03-26 13:48:17 UTC
(In reply to comment #7)
> 
> No, you did.  The xls importer changes now makes the background be
> saved as 9C:9C:FF:FF in both cases.  For better or worse.

I don't remember having fixed that, weird, Alzheimer?


> However the only reason #3 appears fixed is that the image now isn't saved
> the first time around either:

Did you update both goffice and gnumeric?
Comment 9 Andreas J. Guelzow 2012-03-29 00:19:42 UTC
#1 is more complicated than I thought. The type of the meta:user-defined fields gsf:document-parts and gsf:heading-pairs is not a standard type so we don't write a value-type. When we read the info we default to a value type of "string". 

Note that the ODF specs says: "The <meta:user-defined> element has character data content, or depending on the value of the meta:value-type attribute content of type double 18.2, date 18.2, dateTime 18.2, duration 18.2, boolean 18.3.3 or string 18.2."

So we do not need a value-type and can just include character data.

If I change ths code to always output a default value-type of "string" then we still have round trip trouble because for strings we write even an empty string and so the element has a separate end element.
Comment 10 Andreas J. Guelzow 2012-03-29 04:35:40 UTC
Created attachment 210840 [details] [review]
conceptual patch

This is a conceptual patch to fix issue #1. The patch is not correct yet since it doesn't correctly parse the read string in gsf_xml_gvalue_from_str: currently I just cut at every comma, but some of the strings are quoted by "..." and I ignoring those quotes.
Comment 11 Andreas J. Guelzow 2012-03-31 07:11:00 UTC
Created attachment 211018 [details] [review]
proposed patch

This patch fixes issue #1.
Comment 12 Andreas J. Guelzow 2012-03-31 07:17:51 UTC
Note: I do not like this patch but think we can do much better by not storing the gsf-doc-prop-vector as a single meta:user-defined item but by a sequence of such!
Comment 13 Morten Welinder 2012-03-31 15:02:53 UTC
What keeps us from storing the property as a sequence?
Comment 14 Andreas J. Guelzow 2012-03-31 16:03:42 UTC
This is in gsf-opendoc-utils.c. We are using the same way of saving things in .gnumeric and in .ods files. This limits us to what ODF allows us to do, unless we want to use foreign extensions. At this time in libgsf we do not know whether we allow foreign extensions.

There is no meta:user-defined sequence. Using simply several meta:user-defined elements with the same property name does not work since:
"The meta:name attribute specifies the name of a metadata element. The name shall be unique within the domain of <meta:user-defined> elements."

I think we can work around this by using some markers attached to the name.
Comment 15 Andreas J. Guelzow 2012-03-31 17:35:06 UTC
Created attachment 211036 [details] [review]
improved patch

This patch fixes issue #1 by saving a sequence of meta:user-defined elements that are connected by a modified name:

<meta:user-defined meta:name="GSF_DOCPROP_VECTOR:0:gsf:heading-pairs" meta:value-type="string">Worksheets</meta:user-defined>
<meta:user-defined meta:name="GSF_DOCPROP_VECTOR:1:gsf:heading-pairs" meta:value-type="float">2</meta:user-defined>
<meta:user-defined meta:name="GSF_DOCPROP_VECTOR:2:gsf:heading-pairs" meta:value-type="string">Named Ranges</meta:user-defined>
<meta:user-defined meta:name="GSF_DOCPROP_VECTOR:3:gsf:heading-pairs" meta:value-type="float">2</meta:user-defined>
Comment 16 Andreas J. Guelzow 2012-03-31 18:42:34 UTC
I have committed my patch. Issue #1 is now fixed.

diff -u /tmp/obj{,2}.xml doesn't show any differences any more. 

So this bug is fixed?
Comment 17 Morten Welinder 2012-04-02 16:54:59 UTC
This problem has been fixed in the development version. The fix will be available in the next major software release. Thank you for your bug report.