Review Request: fix writing empty strings for attribut xml:id of draw:plugin elements

[Thorsten Zachmann]

> On July 10, 2012, 1:14 p.m., Thorsten Zachmann wrote:
> > plugins/pluginshape/PluginShape.cpp, line 68
> > <http://git.reviewboard.kde.org/r/105503/diff/1/?file=71999#file71999line68>
> >
> >     I think we should also use the xmlid type KoElementReference::Counter here as UUID is just a waste for something like this.
> 
> Friedrich W. H. Kossebau wrote:
>     Not sure, what you mean, the patch _is_ using KoElementReference::Counter :)

Sorry I did not see it as it was moved to the next line.


> On July 10, 2012, 1:14 p.m., Thorsten Zachmann wrote:
> > plugins/pluginshape/PluginShape.cpp, line 119
> > <http://git.reviewboard.kde.org/r/105503/diff/1/?file=71999#file71999line119>
> >
> >     I think this should be changed to !name.isEmpty() as you no longer pass an null string here.
> 
> Friedrich W. H. Kossebau wrote:
>     QString(), which is now used as default parameter, also creates a QString which isNull(). So no need for that change, or?
>

in all other places we check for empty so I think it makes sense to change it for consistency. Looks good for getting committed after fixing this one I would say.


- Thorsten


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/105503/#review15608
-----------------------------------------------------------


On July 10, 2012, 12:39 p.m., Friedrich W. H. Kossebau wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/105503/
> -----------------------------------------------------------
> 
> (Updated July 10, 2012, 12:39 p.m.)
> 
> 
> Review request for Calligra and Inge Wallin.
> 
> 
> Description
> -------
> 
> Some calligratests like interoperability/wordprocessing/oowriter/oow_embede_audio.odt fail on a bad xml:id for the draw:plugin element.
> Reason is that PluginShape currently has a property xmlid, which only gets set on loading. On saving the content of that property is written, even if empty (like for the file above).
> 
> Attached patch fixes this by doing like the rest(?) of Calligra, dropping any read xmlid and generating in any case a new one on saving.
> (Also fixes a bad i18n-with-parameter call and deprecated QString::null usages, or should those be in separate commits?)
> 
> Okay to backport to 2.5?
> 
> PS: In general I think Calligra should try to remember the read xml-ids, it kind-of spoils the idea of ids if those are completely new after saving, even for untouched elements. But that is not in the scope of this fix, which only cares for writing valid ODF.
> 
> 
> Diffs
> -----
> 
>   plugins/pluginshape/PluginShape.h 38c67db 
>   plugins/pluginshape/PluginShape.cpp c1e19ec 
> 
> Diff: http://git.reviewboard.kde.org/r/105503/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Friedrich W. H. Kossebau
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/calligra-devel/attachments/20120710/7f75e753/attachment.htm>