Re: [Libreoffice] [PATCH] DbgAssert error: "HeaderBar::InsertItem(): ItemId already exists"

On Fri, Aug 05, 2011 at 02:44:09PM +0100, Caolán McNamara wrote:
> On Fri, 2011-08-05 at 00:51 +0200, Lionel Elie Mamane wrote:

> > The attached patch solves the error message
> >  Error: HeaderBar::InsertItem(): ItemId already exists
> > that a debug build gives when opening the "Open File" dialog.

> Hmm, patch doesn't apply. Against what version did you create this ?,
> was it a 3-3 or 3-4 or something.

Thanks for taking a look!

Sorry, forgot that "git format-patch" does not include that
metadata. I generated it against branch libreoffice-3-4.

> Conflicts with
> http://cgit.freedesktop.org/libreoffice/libs-gui/commit/?id=5902c3e6bfff3c0162f7dd214fc83c46b4ebb1f1
> which also aimed to silence that IIRC.

Which smells more like a deep "real" fix instead of just doing
minimal changes to silence the warning like my patch.

While we are at it, I find the value of FILEVIEW_SHOW_NONE
suspicious, because it intersects with FILEVIEW_SHOW_ALL. In
particular, if nFlags == FILEVIEW_SHOW_NONE, then
 ( nFlags & FILEVIEW_SHOW_SIZE ) == FILEVIEW_SHOW_SIZE

Which means that code that checks in that way for "should I show the
size" will erroneously conclude that it should.

In libreoffice-3-4, file svtools/inc/svtools/fileview.hxx:

#define FILEVIEW_SHOW_TITLE                     0x0010
#define FILEVIEW_SHOW_SIZE                      0x0020
#define FILEVIEW_SHOW_DATE                      0x0040
#define FILEVIEW_SHOW_ALL                       0x0070
#define FILEVIEW_SHOW_NONE                      0x00A0


In my opinion, we should seriously consider changing
FILEVIEW_SHOW_NONE to

#define FILEVIEW_SHOW_NONE                      0x0080

This could be an incompatible ABI change, but it is semi-safe (or 25%
safe?): code compiled with the new value and doing:

 if ( ( nFlags & FILEVIEW_SHOW_NONE ) == FILEVIEW_SHOW_NONE )

will still take that branch if nFlags contains the _old_ value for
FILEVIEW_SHOW_NONE.


-- 
Lionel