Re: koffice/libs/kotext/opendocument

On Monday, November 08, 2010 10:32:17 you wrote:
> On Monday 8. November 2010 04.33.33 Thorsten Zachmann wrote:
> > Hello Thomas,
> > 
> > On Sunday, November 07, 2010 07:04:18 pm Thomas Zander wrote:
> > >          if (styleManager) {
> > > 
> > > -            styleManager->add(it->second);
> > > +            styleManager->add(odfStyle.style);
> > > 
> > >          } else {
> > > 
> > > -            d->characterStylesToDelete.append(it->second);
> > > +            if (!odfStyle.parentStyle.isEmpty()) { // an auto style 
with a parent. 
> > > +                // lets find the parent and set the styleId of that one 
on the auto-style too. 
> > > +                // this will have the effect that whereever the 
autostyle is applied, it will
> > > +                // cause the parent style-id to be applied. So we don't 
loose this info. 
> > > +                KoCharacterStyle *parent = 
characterStyle(odfStyle.parentStyle, false);
> > > +                if (!parent)
> > > +                    parent = characterStyle(odfStyle.parentStyle, 
true); // try harder 
> > > +               if (parent)
> > > +                    odfStyle.style->setStyleId(parent->styleId());
> > > 
> > >          }
> > > 
> > > +            d->characterStylesToDelete.append(odfStyle.style);
> > 
> > there is a problem with the above code. If a style is in content.xml  and
> > it is a automatic style its parent can be a automatic style in
> > content.xml or a document style in styles.xml. If it is a automatic
> > style and it is in styles.xml its parent can only be a automatic style
> > in styles.xml or a document style in styles.xml. Also a document style
> > can never have an automatic style as parent.
> 
> There is a paragraph in the ODF spec (at least in 1.2) that says automatic
> styles can only have as parent non-automatic styles, aka document styles.
> So, the code makes the assumption that this is the case, if the document is
> not following the spec then indeed funny things may happen ;)

That is the point I have missed. Thanks. You are right parent styles can only 
be document styles.

> > The above code is problematic as automatic styles in content.xml and
> > styles.xml can have the same name.
> 
> That you can name a style in the content and a diffrent style in the styles
> xml the same is new too me, so lets see if that works...

It is true that an automatic style in content.xml and styles.xml can have the 
same name.

> Hmm, if a document style in styles.xml has an odf name that is a duplicate
> of another document style in content.xml, then the design as it was before
> can't work because the line;

However there are only document styles in styles.xml so no duplicate names can 
happen for document styles.

> 
>     addCharacterStyles(shapeContext,
> context.stylesReader().customStyles("text").values(), ContentDotXml |
> StylesDotXml, styleManager);
> 
> adds each of those styles to both collections, and it would drop one style
> from those collections (but add it to the styleManager)
> 
> So, maybe I misundestood? Can we really have two document styles with the
> same odf name in content.xml and styles.xml?

No. Document styles are only in styles.xml.

> If I understood correctly, do you agree I found a problem?

No problem.

There is a small thing that can be improved:

                KoCharacterStyle *parent = 
characterStyle(odfStyle.parentStyle, false);                                                                                                       
                if (!parent)                                                                                                                                                                  
                    parent = characterStyle(odfStyle.parentStyle, true); // 
try harder                                                                                                        

As doument styles are both in characterStyles of styles.xml and content.xml 
only one need to be asked for the parent. So the second line can be removed.

Thorsten