[BUG]: lout::misc::StringBuffer::clear() does not invalidate the string
Well, *that* was a bit of a wild goose chase! Remember I complained of selection problems in ListResources? It was nothing of the kind. Instead test/dw-ui-test was spuriously reporting that things were still selected when everything was in fact unselected. And the cause was a bug in lout::misc::Stringbuffer ! lout::misc::StringBuffer::clear() omits to invalidate the internal cached string, so if you call lout::misc::StringBuffer::getChars() (which updates the cached string and marks it as valid), then lout::misc::StringBuffer::clear() and then *immediately* call lout::misc::StringBuffer::getChars() again, you get the contents of the StringBuffer before you cleared it (because the cached value has not been invalidated), instead of the empty string. D'oh! One-liner patch attached. Regards, Jeremy Henty
On Fri, May 02, 2008 at 01:48:43PM +0100, Jeremy Henty wrote:
Well, *that* was a bit of a wild goose chase! Remember I complained of selection problems in ListResources? It was nothing of the kind. Instead test/dw-ui-test was spuriously reporting that things were still selected when everything was in fact unselected. And the cause was a bug in lout::misc::Stringbuffer !
lout::misc::StringBuffer::clear() omits to invalidate the internal cached string, so if you call lout::misc::StringBuffer::getChars() (which updates the cached string and marks it as valid), then lout::misc::StringBuffer::clear() and then *immediately* call lout::misc::StringBuffer::getChars() again, you get the contents of the StringBuffer before you cleared it (because the cached value has not been invalidated), instead of the empty string. D'oh!
One-liner patch attached.
Good you found it! I know how hard it gets to fix this kind of bugs sometimes. Committed. BTW, does this mean the last selection patch shows no bugs anymore? -- Cheers Jorge.-
On Fri, May 02, 2008 at 12:08:10PM -0400, Jorge Arellano Cid wrote:
On Fri, May 02, 2008 at 01:48:43PM +0100, Jeremy Henty wrote:
lout::misc::StringBuffer::clear() omits to invalidate the internal cached string,
I know how hard it gets to fix this kind of bugs sometimes.
It only appears if you don't add anything to the StringBuffer between clearing it and reading it, so it is hidden in almost all normal use.
BTW, does this mean the last selection patch shows no bugs anymore?
There is one remaining issue with selections but I think it's not to do with my patch. If several resource items are marked selected then only the last one is selected on the FLTK menu widget. I think the problem is that FltkSelectionResource::addItem selects the item widget by calling set_item() on the menu widget, and that's only the right thing to do for a fltk::Choice . For a fltk::Browser you just want to call Widget::set_selected() on the menu item. I guess the fix is to have a virtual hook in FltkSelectionResource and have each of FltkListResource and FltkOptionMenuResource override the hook to do the right thing. *If* I'm right then my selection patch is good as it stands. I'll have a go at a patch and see if it works. I'd be glad of any tips for doing this. The fltk::Menu internals are confusing and I'm also not familiar with the WidgetStack machinery in FltkSelectionResource . Regards, Jeremy Henty
On Fri, May 02, 2008 at 06:15:44PM +0100, Jeremy Henty wrote:
On Fri, May 02, 2008 at 12:08:10PM -0400, Jorge Arellano Cid wrote:
On Fri, May 02, 2008 at 01:48:43PM +0100, Jeremy Henty wrote:
lout::misc::StringBuffer::clear() omits to invalidate the internal cached string,
I know how hard it gets to fix this kind of bugs sometimes.
It only appears if you don't add anything to the StringBuffer between clearing it and reading it, so it is hidden in almost all normal use.
BTW, does this mean the last selection patch shows no bugs anymore?
There is one remaining issue with selections but I think it's not to do with my patch. If several resource items are marked selected then only the last one is selected on the FLTK menu widget. I think the problem is that FltkSelectionResource::addItem selects the item widget by calling set_item() on the menu widget, and that's only the right thing to do for a fltk::Choice . For a fltk::Browser you just want to call Widget::set_selected() on the menu item. I guess the fix is to have a virtual hook in FltkSelectionResource and have each of FltkListResource and FltkOptionMenuResource override the hook to do the right thing. *If* I'm right then my selection patch is good as it stands. I'll have a go at a patch and see if it works.
Oh, that's nasty. There seems to be no way to select multiply items in fltk::Menu. As far as I can see one needs a fltk::Browser object for that. Perhaps the easiest way is to comletely overwrite the addItem() method in FltkListResource? Does FltkListResource need to provide something like submenus? Regards, Johannes
I'd be glad of any tips for doing this. The fltk::Menu internals are confusing and I'm also not familiar with the WidgetStack machinery in FltkSelectionResource .
Regards,
Jeremy Henty
_______________________________________________ Dillo-dev mailing list Dillo-dev@dillo.org http://lists.auriga.wearlab.de/cgi-bin/mailman/listinfo/dillo-dev
On Fri, May 02, 2008 at 09:51:46PM +0200, Johannes Hofmann wrote:
There seems to be no way to select multiply items in fltk::Menu.
If I understand the fltk::Widget docs right, then you just call Widget::set_selected() on as many of the items as you want. That sets the flag that is read by Widget::selected() (which FltkListResource::widgetCallback already calls, so that seems to be the right thing to do). The problem is that the code calls Menu::set_item() to make the selected item the one that the menu displays. This is the right thing for OptionMenus but not for Lists.
Perhaps the easiest way is to comletely overwrite the addItem() method in FltkListResource?
I'm hoping that (a) calling Widget::set_selected() explicitly and (b) adding a hook to decide whether or not to call Menu::set_item() will work. Possibly the *proper* way to it will involve more rewriting as you suggest, but since I don't understand this code very well I am looking for the simplest fix rather than rewriting wholesale stuff I don't understand. Maybe later I'll have the confidence to do a proper revamp.
Does FltkListResource need to provide something like submenus?
It already does. If you apply my patch and run test/dw-ui-test you can double-click on the group item in the browser and the sub-items will appear. Regards, Jeremy Henty
participants (3)
-
jcid＠dillo.org -
Johannes.Hofmann＠gmx.de -
onepoint＠starurchin.org