What’s wrong with this code, part 25 – the answers

  • Article

Yesterday I described a very real bug in some of the Windows UI.

 CControlLayout::CControlLayout(const HWND hWndControl, const HWND hWndDlg)
    : m_hWnd(hWndControl)
    , m_hWndDlg(hWndDlg)
{
    // Get the parent (dialog) rect, and the control rect
    ::GetClientRect(m_hWndDlg, &m_rcRefDlg);
    ::GetWindowRect(m_hWnd, &m_rcRef);
    ScreenToClientRect(hWndDlg, m_rcRef);
}
void ScreenToClientRect(/* [in] */ const HWND hWndClient,                         /* [in/out] */ RECT &rcInOut)
{
    CPoint ptTopLeft(rcInOut.left, rcInOut.top); 
    CPoint ptBottomRight(rcInOut.right, rcInOut.bottom); 
    ::ScreenToClient(hWndClient, &ptTopLeft); 
    ::ScreenToClient(hWndClient, &ptBottomRight); 
    rcInOut.left = ptTopLeft.x; 
    rcInOut.top = ptTopLeft.y; 
    rcInOut.right = ptBottomRight.x; 
    rcInOut.bottom = ptBottomRight.y;
}

And as David Gladfelter pointed out, the root cause of the problem is that the routine calls ScreenToClient.  This works just fine when you’re running on Left-to-Right builds of Windows, but on Right-to-Left languages (Arabic, Hebrew, etc), this code sets the rcInOut.left to the wrong location.

It turns out that MSDN has a warning that is explicitly about this kind of problem:

For example, applications often use code similar to the following to position a control in a window:

Copy Code

 // DO NOT USE THIS IF APPLICATION MIRRORS THE WINDOW

// get coordinates of the window in screen coordinates
GetWindowRect(hControl, (LPRECT) &rControlRect);  

// map screen coordinates to client coordinates in dialog
ScreenToClient(hDialog, (LPPOINT) &rControlRect.left); 
ScreenToClient(hDialog, (LPPOINT) &rControlRect.right);

This causes problems in mirroring because the left edge of the rectangle becomes the right edge in a mirrored window, and vice versa. To avoid this problem, replace the ScreenToClient calls with a call to MapWindowPoints as follows:

Copy Code

 // USE THIS FOR MIRRORING

GetWindowRect(hControl, (LPRECT) &rControlRect);
MapWindowPoints(NULL, hDialog, (LPPOINT) &rControlRect, 2)

It turns out that this is explicitly the mistake that was made in the code.  The good news is that the “Use this for mirroring” code listed in the article is exactly the fix necessary to solve this problem.

 

As I mentioned, David Gladfelter was the first person to pick up the problem, kudos to him!