[XForms] Question #2: Relying on menu subitem IDs after fl_delete_menu_item().
jason.cipriani at gmail.com
Fri Jul 4 17:46:47 EDT 2008
To subscribers of the xforms list
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.
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
sp->mval[ n ] = strtol( p + 2, &eptr, 10 );
while ( *eptr && isspace( ( int ) *eptr ) )
if ( *eptr )
memmove( p, eptr, strlen( eptr ) + 1 );
*p = '\0';
The fix is to not remove the "%x" from the string, changing it to just this:
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.
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;
+ 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.
2) We know nobody has been using %x to set the IDs in regular menus,
since it was never supported.
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
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
Incidentally, I've also noticed that, every once in a while, fdesign
strips the %x from the end of my menu strings. I am not sure why, or
what triggers it. It's happened to me twice out of about 15 saves so
To unsubscribe, send any message to
xforms-leave at bob.usuhs.mil or see:
List Archive: http://bob.usuhs.mil/pipermail/xforms and
More information about the Xforms