Style objects for applications and filters

[Inge Wallin]

On Sunday, May 04, 2014 06:18:45 Thorsten Zachmann wrote:
> On Monday, April 28, 2014 10:31:45 PM Inge Wallin wrote:
> >  * Represent styles as text attributes wherever possible so that they can
> >  be
> > 
> > complete, i.e. not throw away style data.
> 
> I'm not conviced that this is a good idea. We tried to do that with the
> KoUnavailShape to prevent what we can't save but as far as I remeber LO and
> OO where unfortunately not able to load the stuff we saved back.

I analyzed that later and that was because we saved back all files but did not 
save back empty subdirectories (used in charts if I remember correctly). I 
created a patch for that and I think it's committed but I'm not 100% sure.

> Also saving back styles might result in unwanted behaviour. See the
> following example:
> 
> User 1 creates a document with styles in LO.
> User 2 loads the document with calligra.
> User 2 copy and pastes something with unsupported formating options.
> User 2 formats it as he wants it to be. (however as he copied something he
> can't see he also has a format in there which will result in something
> different then expected when loaded)
> User 2 save
> User 1 loads in LO and the formating of the pasted text is not what User 2
> wanted.
> 
> Sure if User 2 only fixes a typo this is not a problem but if a user really
> works with a document it can happen in quite a lot supricing side effects
> the user can't forsee and therefore I against this.

This scenario is indeed possible and not good.  But I believe that it would be 
less common than the following scenario (given your suggestion to not save 
unsupported formatting):

 User 1 creates a document with styles in LO.
 User 2 loads the document with calligra.
 User 2 copy and pastes something with unsupported formating options and the 
unsupported formatting is removed by calligra.
 User 2 formats it as he wants it to be
 User 2 save
 User 1 loads in LO and the formating of the pasted text is now different from 
the rest of the text in the document.

My reasons for believing this is:
 * Calligra does already support almost all formatting and any formatting that 
we don't support is pretty exotic and therefore almost certainly by design in 
the document.
 * Most documents only contain a limited set of styles. And almost all of the 
text in a typical document is going to use pretty standard formatting - which 
calligra already supports.
 * So any unsupported formatting is something that User 1 wants.

And even if your scenario plays out then removing some formatting from a lot 
of text is going to be much easier than reapplying it to the specific parts 
that need it (removing formatting that isn't already there is a null 
operation).

And funnily enough I came across a bug of exactly this type yesterday on the 
documentliberation-discuss mailing list: 
https://bugs.freedesktop.org/show_bug.cgi?id=77855
As you can see from the screenshots this is quite serious and only one 
example.

> There are interdependencies between styles that can't be saved by in the way
> you described as it will need to update references and so on. Also I could
> not find something in the specification that says a reader/writer should
> support stuff  he does not understand.

Hmm, what interdependencies between styles do you mean? I know that there are 
interdependencies between individual attributes (e.g. you cannot get border-
left from one style and border-right from its parent) but not between styles.

But fixing this has nothing to do with being complete per se. It's orthogonal 
to it. I do think that we should create a list of interdependent attributes 
and make sure that we have a generic way of handling it.

> >  * Implement a binary interface for some (in the long run all) attributes
> >  so
> > 
> > that we retain the advantages of that (mostly speed).
> 
> If we go the route you propose then I would propose to add the new stuff as
> addition to what is there.

Agreed. Implementation wise we should throw away as little working code as 
possible.

> >  * Because all our apps use DOM parsing now (even if stream reading is
> >  used
> > 
> > under the hood) and many filters use stream parsing, I want to provide
> > both
> > variants of the loadOdf() method.
> 
> More code means more bugs so I'm not really convinced here.  At least quite
> a lot of testing for that is needed.

Yes, testing is important.  I am willing to write test code to test retaining 
properties of styles in different roundtrip scenarios. And for visuals we have 
cstester.

Note, though, that the alternative to doing this is not *not* writing the new 
parsing code.  It's writing even more code because the stream parsing API is 
already used by the filters. So instead of just adding 
load/saveOdf(KoStreamReader &) to existing objects we will have to write 
completely new objects to store the same thing.  For already existing examples 
of this, see all classes in filters/libodf2/ which do not start with KoOdf.  
There is a description of it in the README.

And I am not suggesting that the code using these objects already (i.e. the 
applications) move from dom parsing to stream parsing. I only want to reuse 
code that is already working well in new situations - in this case the filters.

But again I think this is two discussions mixed into one.  The two different 
issues are:
1. Shall we introduce a policy to keep also unsupported formatting and save it 
back?  This will demand a slightly different base implementation of styles and 
will indeed demand a lot of testing.
2. Shall we allow two different load/saveOdf() API's in the already existing 
objects so they can be also used in the filters?

Issue 1 is the big one.  Issue 2 is much smaller and is what you are replying 
to in the paragraph just above. 

> So I'm not really convinced this is a good idea.

Noted.  Any opinions from other people?

	-Inge

> 
> Thorsten
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/calligra-devel/attachments/20140504/c324a2f2/attachment.htm>