On 14.06.2010 17:49, Malte Timmermann wrote:


Björn Michaelsen wrote, On 06/14/10 16:09:
Am Mon, 14 Jun 2010 15:11:57 +0200
schrieb Malte Timmermann<malte.timmerm...@sun.com>:

Hi Bjoern,

Björn Michaelsen wrote, On 06/14/10 13:34:
Hi all,

while testing the new SfxItemSets in cws new_itemsets I came across
a few interesting uses of itemsets which I had considered to be
illegal (but I might be wrong there). I assumed:
- non-void items should only be put at the same which id in a set
as the which id on the item itself.
- void items should always have a which id of 0 and are allowed to
be put anywhere.
Who says they would need to have which ID 0 ?

Well, as I said I can be wrong here. But a SfxVoidItem with which id
set to 0 is interpreted as "disabled". However, some functions only
test for the type being SfxVoidItem, while others test only for the
which id being 0. Thus having a "half-disabled" item might lead to
"interesting" results. See for example:

http://svn.services.openoffice.org/opengrok/xref/DEV300_m82/svl/source/items/itemset.cxx#542
(SfxItemSet::GetItemState)
http://svn.services.openoffice.org/opengrok/xref/DEV300_m82/svl/source/items/itemset.cxx#589
(SfxItemSet::Put)
http://svn.services.openoffice.org/opengrok/xref/DEV300_m82/svl/source/items/itemset.cxx#2040
(SfxItemSet::DisableItem)

So, when you used SfxVoidItems, did you intend them to be interpreted
as "disabled"?

No - but it doesn't matter, because I only use them for character
attributes (single items), not for paragraph attributes (item set).

They probably only get into the item sets because the SfxItemPool has
default items, I don't think that I put them in some SfxItemSet explicitly.

So this is code broken (the code in the ItemSet, not yours). If some code in SfxItemSet just checks for SfxVoidItems to decide whether it is a disabled item, while other code just checks for ID == 0, the problem is already there. It's also an unnecessary complexity to require the check of two attributes (type *and* ID). The next interesting question is what should happen if items derived from SfxVoidItems are used ...

So perhaps coding "disabled" as SfxVoidItem is a severe design flaw. So it would be better if disabling (and checking for disabed state) should be possible only by using explicit methods from SfxItemSet. If the implementation requires to code that by storing a special item, this item should be a private class in SfxItemSet so that it can't be used outside. The problem with that approach is that it requires a non-trivial amount of work. :-(

Of course they get into the SfxItemPool...

BTW - they are also used in the binary document format, SfxItemSet
load/store, so I also don't know what would happen here if the IDs where 0.

So we are lucky that this format is doomed. :-)

Regards,
Mathias

--
Mathias Bauer (mba) - Project Lead OpenOffice.org Writer
OpenOffice.org Engineering at Sun: http://blogs.sun.com/GullFOSS
Please don't reply to "nospamfor...@gmx.de".
I use it for the OOo lists and only rarely read other mails sent to it.

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@openoffice.org
For additional commands, e-mail: dev-h...@openoffice.org

Reply via email to