On Thu, Nov 22, 2012 at 08:52:20AM +0100, Maciej Rumianowski wrote: > I have hit a problem that i don't fully understand. Disabled items are > SfxVoidItem with Which 0, but inserted with different one. State of Item is > check with TYPE Macro as below > > > if ( (*ppFnd)->Type() == TYPE(SfxVoidItem) ) > > return SFX_ITEM_DISABLED; > > > > So it just type that decides about disabled state of item. I've changed > places where conditions like > > > if( !(*ppFnd)->Which() ) > > > to > > > if ( (*ppFnd)->ISA(SfxVoidItem) )
Yes, there are quite a few evil cornercases lurking there. Before changing those conditionals, I think it is a good idea to just add assertions that make sure these assumptions are kept consistent all the time. A start would be to check that when an item is put in the set, to check: - If it has a non-zero Which-Id - and is not a SfxVoidItem any client code that does something different (even explicitly putting a SfxVoidItem), is abusing the API and should be fixed by making it call DisableItem instead). Once you are sure that is cleaned up, one can carefully go on about changing the conditions. Some care is still needed as in theory, someone might have e.g. put a non-zero, non-SfxVoidItem and later set the WhichId on the item to 0. Of course, we then need to send out a team to hunt down those responsible and whack them with a large trout in their sleep, if we ever come across such a case. So, to change the conditionals, we need to make sure the state is consistent. Thus: - adding the asserts - run e.g. subsequentcheck to see if they every pop something up After that: Carefully start migrating the conditionals. Does that make sense? Best, Bjoern _______________________________________________ LibreOffice mailing list LibreOffice@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/libreoffice