[XForms] Question #2: Relying on menu subitem IDs after fl_delete_menu_item().
Jens Thoms Toerring
jt at toerring.de
Sat Jul 5 08:04:46 EDT 2008
To subscribers of the xforms list
Hi Jason,
On Fri, Jul 04, 2008 at 05:46:47PM -0400, Jason Cipriani wrote:
> On Fri, Jul 4, 2008 at 4:37 PM, Jason Cipriani <jason.cipriani at gmail.com> wrote:
> > On Fri, Jul 4, 2008 at 6:57 AM, Jens Thoms Toerring <jt at toerring.de> wrote:
> >> To get around that you have to change the file lib/menu.c.
> >> I append a version for 1.0.90 that hopefully works but I
> >> didn't test the changes with that version, just my newer
> >> one. Then add to lib/private/menu.h
> >
> > Since this change, the functions fl_set_menu_item_mode and
> > fl_get_menu_item_mode no longer work unless the menu item's index is
> > the same as it's value. It doesn't matter if you specify an index or a
> > value to these functions -- if the two are different, they are
> > ignored. I suspect that something internally is inappropriately mixing
> > item indices and values, and had always relied on them being the same.
>
> I've found the problem, and a partial solution to it (see below, I've
> also provided a possible full solution), although I'm not sure if the
> solution breaks anything.
>
> SOLUTION #1:
>
> In the new %x handling code in addto_menu, around line 357 there is
> this bit of code that removes the %x from the string after processing
> it:
>
> else
> {
> sp->mval[ n ] = strtol( p + 2, &eptr, 10 );
> while ( *eptr && isspace( ( int ) *eptr ) )
> eptr++;
> if ( *eptr )
> memmove( p, eptr, strlen( eptr ) + 1 );
> else
> *p = '\0';
> }
>
> The fix is to not remove the "%x" from the string, changing it to just this:
>
> else
> {
> sp->mval[ n ] = strtol( p + 2, &eptr, 10 );
> }
>
> The reason this problem was occurring is because every time you
> display a menu, it uses the setpup stuff to create a popup menu to
> display. This popup menu is set up in do_menu around line 111 of
> menu.c. All of the popup menu functions take item IDs, not indices.
> When you use %x to modify the menu's ID, this causes sp->mval[n] to
> hold the ID, and so all of the MENU functions know about the ID.
> However, since the %x is removed from the string, when the popup is
> created that corresponds to the menu, the item IDs are not propagated
> to the popup menu -- therefore the MENU functions know about the item
> ID but the POPUP functions do not. Leaving the "%x" in the string
> allows the ID to be passed through fl_addtopup() and then the correct
> IDs are set in the popup menu, and everything seems to work.
>
> SOLUTION #2:
>
> The reason the above fix isn't complete is that it only applies when
> you've used %x to set the IDs. The problem solved by the above
> solution is related to the new feature of parsing %x in menu strings
> but is not related to removing menu items. However, removing menu
> items does cause a problem if you haven't used %x at all that the
> above solution doesn't fix. If that makes sense.
>
> Anyways if you do not use %x then the menu item IDs are, initially,
> the item indices. If you remove a menu item, then indices change but
> the IDs do not. However, the IDs in this case are never passed to
> fl_addtopup at all, and so the POPUP menu uses the item INDICES
> instead -- which no longer correspond to the IDs since an item was
> removed. This is related to the problem when using %x in that it's
> also caused by inconsistency between the item IDs in menu.c and the
> item IDs in xpopup.c.
>
> The root cause of all of these problems is that do_menu does not set
> the popup menu item IDs to the sp->mval[i] values. All menu ID
> information is lost in the popup. AFAICT, there is no API call to
> change popup menu item IDs, and therefore there is no easy way for
> do_menu() to pass ID information to the popup menu when it sets up the
> menu with fl_addtopup().
>
> One solution for this is to continue removing the %x strings in
> addto_menu, just as you are doing now, but when creating the popup
> menu, create temporary strings with a %x concatenated onto the end of
> them along with the corresponding ID. Something like this in do_menu
> around line 111:
>
> + char tempstr[512];
> + snprintf(tempstr, sizeof(tempstr), "%s%%x%i", sp->items[i], sp->mval[i]);
> + fl_addtopup(sp->menu, tempstr);
> - fl_addtopup(sp->menu, sp->items[i]);
>
> This works fine but it does change the behavior of some other stuff.
> Specifically, it causes fl_get_menu to return the item ID and NOT the
> item index as previous. Meaning that you can just do this:
>
> int value = fl_get_menu(menu);
>
> Rather than having to do this:
>
> int value = fl_get_menu_value(menu, fl_get_menu(menu));
>
> Although that now also means that if you're passing the result of
> fl_get_menu() to other menu functions, you'll need to convert the
> value to an index first:
>
> int index = fl_get_menu_item_from_value(menu, fl_get_menu(menu));
> fl_delete_menu_item(menu, index); /* for example */
>
> It's important to note, though, that the above changes in behavior
> would ONLY happen if the IDs and values are not the same, which can
> ONLY happen if you've either used %x to change the IDs, or you've
> removed a menu item. Therefore I believe using the above solution
> (constructing a temporary string with a %x on the end) will not
> significantly affect any existing applications because:
>
> 1) We know nobody has been removing menu items, since
> fl_delete_menu_item didn't work at all.
It did work, but only for menus created with fl_addto_menu(),
not for thise created with fl_set_menu_entries().
> 2) We know nobody has been using %x to set the IDs in regular menus,
> since it was never supported.
It's even documented as not to work.
> 3) The only people who may experience changes in behavior are people
> that had a previously meaningless "%x" in their menu item text, and
> now it means something. However, your addto_menu change already
> affected these people, the tempstr thing above doesn't add any new
> issues.
>
> What do you think about solution #2? Do you see any way it could
> interact with regular popup menus at all? What about fdesign? Things
> like that?
I think all your ideas are very reasonable and #2 is the way
I will implement it in the new version. Thanks for doing my
work;-)
That means:
a) A menu ID can be associated with a certain menu item using
"%xn" in the string for the text of the menu item. Note that
'n' must be a number larger than 0.
b) If no menu ID is assigned explicitely it gets its ID from a
counter that is incremented for each menu item added (start-
ing from 1). This number is never re-used, even when menu
items get deleted. The only exception is that on a call of
fl_clear_menu() also the counter gets reset to 1.
c) fl_get_menu() now returns the menu ID
d) To all functions expecting an item number the ID must now
be passed.
e) If two menu items receive the same ID the results are un-
specified, so the user must make sure that this never
happens.
f) If a menu item gets delete via fl_delete_menu_item() all
other items keep their numbers, even if no menu IDs have
been set for them.
That is how things work for normal popups and it probably
wouldn't be a good idea to do it differently for menus. The
only deviations are for the case that menu items get deleted,
which can't be done for popups.
That also means that both the functions fl_get_menu_value() and
fl_get_menu_item_from_value() can disappear again (or the latter
go back to being a local only function).
Does anybody see any problems with this approach?
What I will also try to implement is having a callback function
for each menu item. For menus created with the fl_set_menu_entries()
function you can already do that, so it looks reasonable to me also
to have that ability for all menus. I think I will allow a "%f" in
the menu string and change the functions fl_set_menu(), fl_adddto_menu()
and fl_replace_menu_item() to be variadic functions (that should
introduce compatibilty problems) so that pointers to functions of
type FL_PUP_CB can be passed on.
Finally, I will have another look if it's possible to get rid of
the asymmetry between menus created with calls of fl_addto_menu()
and those created by a single call of fl_set_menu_entries().
Having them behave differently in several respects doesn't look
too good to me (and the differences at least need to be documen-
ted).
Best regards, Jens
--
\ Jens Thoms Toerring ________ jt at toerring.de
\_______________________________ http://toerring.de
_______________________________________________
To unsubscribe, send any message to
xforms-leave at bob.usuhs.mil or see:
http://cweblog.usuhs.mil/mailman/listinfo/xforms
List Archive: http://bob.usuhs.mil/pipermail/xforms and
http://bob.usuhs.mil/mailserv/list-archives/
Development: http://savannah.nongnu.org/files/?group=xforms
More information about the Xforms
mailing list