GNOME Bugzilla – Bug 348780
Images used as background in graphs are not serialized
Last modified: 2008-08-03 10:13:26 UTC
This bug is reproducible with libgoffice CVS-HEAD and AbiWord CVS-HEAD on FC5. The same steps work fine with AbiWord 2.4.5, though. Steps to reproduce: - Load http://www.abisource.com/~uwog/abiword-testsuite/impexp/abw/simple-chart.abw - Right-click on the chart and select 'Format Object' - With 'Graph' selected choose 'Image' from the Fill combo - Press the Select button and choose an image (e.g. /usr/share/pixmaps/bug-buddy.png) - Press Apply Backtrace: Program received signal SIGSEGV, Segmentation fault.
+ Trace 69648
Thread NaN (LWP 1912)
I'm quite surprised that it works fine with abiword-2.4.5 since we never wrote the code to save the image with the style. I suspect you just mean it does not crash, but the image should not be there in abiword. I can't reproduce the bug at the moment, but I did not update recently... I'll have a closer look ASAP.
OK, I can now reproduce.
(In reply to comment #1) > I'm quite surprised that it works fine with abiword-2.4.5 since we never wrote > the code to save the image with the style. I suspect you just mean it does not > crash, but the image should not be there in abiword. Indeed, I meant it didn't crash. Sorry for the confusion.
Created attachment 70712 [details] [review] proposed patch fixing the crash is straightforward. We just need to test if the GImage* arg is NULL in go_image_get_pixbuf. But this patch also adds support to serialization of the image. I don't know if it is a good idea to split the code between go-image.c and gog-style.c, so this patch needs some review. Jody, your opinion? I did not test the sax importer.
Now that we have GODoc I think it's time to solve this properly. To date we have never had an object to manage GOImages. We should really be able to store a set of images as named objects at the document level. Then refer to those images in the styles and other sources by name. That will let us solve the issues with inline and reference images. At a minimum get the crash test, and the style property segments into cvs. Then we can discuss how to store the image itself in the document, and how to reference that elsewhere.
Created attachment 71128 [details] [review] commited patch At least, abiword should not crash anymore. Using GODoc to save the pixels seems a great idea, but we must be careful to not break anything.
renaming and changing the severity since the crash is fixed.
Unfortunately pushed off into 0.5.x But I'd like to see this tackled early
The main issue to solve is having GODoc manage the images. This is all but trivial IMHO.
Images pushed to GODoc (in trunk), but still not serialized.
Created attachment 114426 [details] [review] makes things mostly work Jody, I know you don't like the GOImage::saved hack, but it is the simplest solution. Another way would be to save all images at the end of the document. Copying to another workbook in the same process works, but not yet inter-process which uses the (not yet implemented) DOM model for loading the data.
Created attachment 114541 [details] [review] gnumeric side patch
Created attachment 114542 [details] [review] the good one, I hope Jody, please review, and specially have a look at the comment in go-doc.c, function load_image_data. When pasting use SAX, images will also survive inter process copying.
Created attachment 115770 [details] [review] last patch
Created attachment 115771 [details] [review] the associated gnumeric patch
Fixed, although in some cases we might end with two or more instances of the same images in memory (inter-process clipboard operations with identical images in the two documents), IMHO a very minor issue.