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 737039 - t6101-mathfuns-ods.pl fails
t6101-mathfuns-ods.pl fails
Status: RESOLVED FIXED
Product: Gnumeric
Classification: Applications
Component: import/export OOo / OASIS
git master
Other All
: Normal normal
: ---
Assigned To: Andreas J. Guelzow
Jody Goldberg
Depends on:
Blocks:
 
 
Reported: 2014-09-20 18:46 UTC by Morten Welinder
Modified: 2014-09-25 04:30 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
proposed patch (4.33 KB, patch)
2014-09-25 03:53 UTC, Andreas J. Guelzow
none Details | Review

Description Morten Welinder 2014-09-20 18:46:11 UTC
t6101-mathfuns-ods.pl currently fails, see below.  That failure is caused
by an attribute order inversion that probably isn't serious but certainly
is curious.

This was in all likelihood caused by me changing pango_attr_list_insert_before
into pango_attr_list_change in odf_text_p_apply_pango_attribute.

We needed that change to prevent attribute ranges from becoming fragmented.
We want, say, 0-10 to be bold, not 0-5 and 5-10.  pango_attr_list_insert_before
is too primitive here.

But why did we need the "_before" part anyway?  Are we saving or loading
things in the wrong order?




welinder@toshiba ~/gnome/gnumeric/test $ ./t6101-mathfuns-ods.pl --verbose
-------------------------------------------------------------------------------
t6101-mathfuns-ods.pl: Check the ods exporter.
# ../src/ssconvert 'mathfuns.ods' 'mathfuns.gnumeric'
# ../src/ssconvert 'mathfuns.ods' 'mathfuns-new.ods'
# ../src/ssconvert 'mathfuns-new.ods' 'mathfuns-new.gnumeric'
--- mathfuns.xml	2014-09-20 14:41:19.718053191 -0400
+++ mathfuns-new.xml	2014-09-20 14:41:19.722053215 -0400
@@ -353,7 +353,7 @@
         <gnm:Selection startCol="0" startRow="0" endCol="0" endRow="0"/>
       </gnm:Selections>
       <gnm:Objects>
-        <gnm:CellComment ObjectBound="H30" ObjectOffset="1 0 1 0" Direction="17" Print="1" Author="welinder" Text="welinder:&#10;The formula here is carefully set up to check actual equality, not almost-equality." TextFormat="@[color=00x00x00:0:93][bold=1:0:9]"/>
+        <gnm:CellComment ObjectBound="H30" ObjectOffset="1 0 1 0" Direction="17" Print="1" Author="welinder" Text="welinder:&#10;The formula here is carefully set up to check actual equality, not almost-equality." TextFormat="@[bold=1:0:9][color=00x00x00:0:93]"/>
       </gnm:Objects>
       <gnm:Cells>
         <gnm:Cell Row="0" Col="0" ValueType="60">MATH AND TRIGONOMETRY FUNCTIONS</gnm:Cell>
diff died due to signal 1
Comment 1 Andreas J. Guelzow 2014-09-20 23:01:43 UTC
odf_text_p_apply_pango_attribute is effectively called from odf_text_span_end so using pango_attr_list_change simply will not work:

spans can be nested, so when we set the style of a span we cannot simply remove all styles that we set for included spans just because they are of the same type. In fact we need to keep those!

I don't see how this has anything to do with fragmenting styles. If the spans are fragmented then using _before does not fix it, but really the problem is that one should never have written those fragmented spans.
Comment 2 Andreas J. Guelzow 2014-09-20 23:04:01 UTC
I am considering raising the severity to major because we are now no longer correctly read a span of bold text that contains a subspan of normal weight text, or any such situation!
Comment 3 Morten Welinder 2014-09-21 01:36:05 UTC
Don't worry about severity -- this is unreleased.

If we change back, t6514 fails with this problem.  Note, that italic
has been split in two.

-        <gnm:CellComment ObjectBound="B15" ObjectOffset="1 0 1 0" Direction="17" Print="1" Author="Morten Welinder" Text="Rich text found here on this line.&#10;Very rich text.&#10;" TextFormat="@[italic=1:0:4][bold=1:5:9][underline=single:10:15][underline=double:16:20][strikethrough=1:21:23][underline=low:29:33][italic=1:35:44][bold=1:40:49]"/>
+        <gnm:CellComment ObjectBound="B15" ObjectOffset="1 0 1 0" Direction="17" Print="1" Author="Morten Welinder" Text="Rich text found here on this line.&#10;Very rich text.&#10;" TextFormat="@[italic=1:0:4][bold=1:5:9][underline=single:10:15][underline=double:16:20][strikethrough=1:21:23][underline=low:29:33][italic=1:35:40][italic=1:40:44][bold=1:40:44][bold=1:44:49]"/>
Comment 4 Andreas J. Guelzow 2014-09-21 01:43:53 UTC
To save this in an ODF file we have to split the italic into two parts: we cannot have overlapping runs there.

When we read it we may need to do the _before first (in case the generator used subspans of the same type, note that we do not and I haven't seen them with LO either but the standard allows them) and then do some normalization afterwards.
Comment 5 Morten Welinder 2014-09-24 16:46:22 UTC
Andreas, can I ask you to come up with a patch?

I would like to get a new release out soon due to (unrelated crasher)
bug 737261.
Comment 6 Andreas J. Guelzow 2014-09-25 03:53:56 UTC
Created attachment 287035 [details] [review]
proposed patch

This patch continues to use pango_attr_list_change by first collecting all spans (determining the endpoints of the spans) and then applying them in order as they appear.
Comment 7 Andreas J. Guelzow 2014-09-25 04:30:17 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.