[calligra] libs/kotext/styles: Fix saving line-height to ODF. Just because the property exists doesn't mean it's valid.

Sebastian Sauer mail at dipe.org
Tue Jul 12 13:17:12 BST 2011


Pierre wrote:
> On Friday 08 July 2011 18:51:17 Sebastian Sauer wrote:
>> Git commit 9f017cf21d343c1408214499bd878b987d8daf6f by Sebastian Sauer.
>> Committed on 08/07/2011 at 18:43.
>> Pushed by sebsauer into branch 'master'.
>> 
>> Fix saving line-height to ODF. Just because the property exists doesn't
>> mean it's valid.
>> 
>> M  +3    -3    libs/kotext/styles/KoParagraphStyle.cpp
>> 
>> http://commits.kde.org/calligra/9f017cf21d343c1408214499bd878b987d8daf6f

[...]

> I don't disagree with the idea of this change, but this is not really a
> bug in calligra itself.
>
> I will fix the unit test you broke, by adding a specific exception for
> this property. But the following code in OpenDocument relax ng is wrong :
> 
> <attribute name="fo:line-height">
> <choice>
> <value>normal</value>
> <ref name="nonNegativeLength"/>
> <ref name="percent"/>
> </choice>
> </attribute>
> 
> Since nonNegativeLength can be zero, it means that 0 is a valid
> line-height... Still, the specification doesn't tell us what it means.

First the fix above makes sure we do NOT save ALWAYS as percent. Second it 
makes sure all out code is consistent. Layouting and Words ALL do expect !=0 
(please grep yourself).

That the specs are using nonNegativeLength does NOT mean that zero is 
included. The specs are NOT that clever. The only way to check what ranges are 
allowed and/or processed, what happens with invalid ranges, what are the 
default values, etc. pp. is trying MSO and OO.org and looking at there source-
code (OO since MSO doesn't have that possibility). That's what I did. The 
allowed minimum value is 0.02" what is btw EXACTLY what I wrote down last year 
as comment and which was ignored by those who broke that logic again. Please 
see in KoParagraphStyle.cpp line 1247 which says;

// fixed value is between 0.0201in and 3.9402in

You will not find that in the specs or in the schema. The specs and the schema 
do NOT handle ranges, default values, exceptions, corrections, etc. pp. You 
can also try yourself loading ODT's with different values into OO.org (I btw 
did that last year and added the comment at line 1247 for that and already 
fixed it before but seems someone keeps to break it).

Now I like to know what unittest I broke?



More information about the calligra-devel mailing list