[bug 4004] MenuItemInfo vs GetMenuString - Try 2

Ann & Jason Edmeades us at edmeades.me.uk
Mon Dec 26 04:57:04 CST 2005


>>The fact that you need to intorduce the txtWasAllocated flag suggests that
>>something is wrong. Also MF_OWNERDRAW certainly does not qualify as a
MF_STRING
>>alias. I'll try to play with your test case and see if I can find a better
fix.

The particular problem in the bug report was that the ownerdraw can indeed
have an associated string value (which is converted to Unicode if necessary)
which is kept across the menu item being converted to a String or Ownerdraw
type, and our code wouldn't allow returning the string value nor did it save
a copy of it in the ownerdraw case.

It also highlighted problems with the code where we wouldn't free up the
allocated storage in one of the cases even for the string type, and once you
added the ownerdraw type into the decision making about freeing then it was
a lot simpler and cleaner to save whether the memory was allocated or not. I
don't doubt you could do it without the flag, but it would also be more
complex checks in each case. 

>You are correct. The real problem is that the text field should not be
>used anymore for bitmaps handles after the hbmpItem field was added just
>for that. This is proven by the fact that strings cannot only be
>combined with owner draw menu, but also with bitmaps and even
>separators.

Hmmm... As was found from the tests once I checked the rc, you cant convert
a string type to a bitmap type directly, although I didn't test adding a
valid bitmap handle. My tests haven't proved (or attempted to prove) whether
you can have a string attached to a bitmap type, but reading the MSDN it
states that a menuitem which is a bitmap has the low-order word of the
dwTypeData member as the bitmap handle, and hence implies you couldn't have
an associated text (as that is normally pointed to via the same field).
However, since Windows also copies the value, they too may have the
equivalent of the internal 'text' field which may or may not get cleared, so
perhaps this is wrong. It was outside the scope of what I was trying to fix.

>I am too busy celebrating vacation atm, but I hope to send in a patch
>with extensive tests somewhere tomorrow or so.

Sure, thanks - I'll watch out for it. Can you make sure you incorporate the
specific tests for the bug report in it too please.

Jason

PS While I think about it, I've also fixed another menu issue as well (bug
3095), and have attached a sample patch to the bug report which includes
this and the changes to fix and test that one, and if you have an interest
in the menu code you might want to check that out as well. I was going to
submit it once this one got into cvs.





More information about the wine-devel mailing list