What’s wrong with this code, part 24 – the answer

  • Article

In my last post, I included a snippet from an MSDN article written by Kenny Kerr. 

The snippet was pretty straightforward, but had a subtle bug in it:

 CRect rectangle;
VERIFY(m_splitButton.GetWindowRect(
    &rectangle));

TPMPARAMS params = { sizeof(TPMPARAMS) };
params.rcExclude = rectangle;

CMenuHandle menu = m_menu.GetSubMenu(0);

VERIFY(menu.TrackPopupMenuEx(TPM_LEFTBUTTON,
    rectangle.left, rectangle.bottom,
    m_hWnd, &params));

 

The problem was that on Bidi localized systems the popup menu is located in the wrong location.  The TrackPopupMenuEx API takes screen coordinates for the popup menu and on a LTR system creates the popup window with the top left corner of the window at that screen coordinate.  The problem here is that on an RTL system the top right of the menu is located at the screen coordinates.  The good news is that the fix is relatively simple:

 CRect rectangle;
VERIFY(m_splitButton.GetWindowRect(
    &rectangle));

TPMPARAMS params = { sizeof(TPMPARAMS) };
params.rcExclude = rectangle;

CMenuHandle menu = m_menu.GetSubMenu(0);

VERIFY(menu.TrackPopupMenuEx(TPM_LEFTBUTTON,
    ((GetWindowLong(m_hWnd, GWL_EXSTYLE) & WS_EX_LAYOUTRTL) != 0 ? rectangle.right :  rectangle.left), rectangle.bottom,
    m_hWnd, &params));

Before I posted this blog post, I asked the author (who also commented on the previous post) about it.  His response was:

As I tried to trim the sample to the bare minimum I didn’t bother with this. You can use GetSystemMetrics to get the proper alignment, but I’m not sure this constitutes a “nasty bug” so you may be after something else.

IMHO Kenny’s right – code samples are intended to be the bare minimums, there are tons of differences between what you do in a code sample and what you do in production code – support for RTL languages is one of those taxes that get left behind when writing samples.

The reason I wrote the post wasn’t to pick on Kenny or MSDN magazine.  Instead it was to point out the fact that popup menus tied to UI elements don’t work the way you expect on mirrored builds, leading to subtle display issues.

 

 

Kudos and mea culpas:

First off, my phrasing was incorrect – the bug wasn’t “nasty” it was subtle and not at all obvious.

Kenny’s comment was the first to mention the RTL issue, but to be fair, he and I had exchanged emails last week about this issue so I don’t feel good in giving him the kudos.  Instead I’m going to give it to Ryan who correctly pointed out that the snippet assumes LTR order.

 

There are some other interesting potential issues like Maciej Rutkowski pointing out that the UI might misbehave at the bottom of the screen (a good point).