c00000fd
c00000fd

Reputation: 22327

MFC's dialog-based app title bar highlighting visual artifacts on Windows 10 (i.e. bugs in CDialogEx)

I'm not sure why am I getting this visual artifact?

Here's how to repro:

I'm using Visual Studio 2017 Community. Create a new C++ -> MFC project:

enter image description here

Then specify "dialog based":

enter image description here

Then build as "Debug" x86 app and run it.

So I'm running it on Windows 10.

When this dialog-based process has focus, it looks as I would expect it:

enter image description here

but if I switch keyboard focus to some other app (by clicking on it), this dialog-based process still retains its title bar color:

enter image description here

I'm not sure if it's just a matter of a visual glitch or if there's a deeper mess-up with the window message handling. How do I correct it? (This wasn't an issue with older MFC projects.)

Upvotes: 4

Views: 1683

Answers (3)

Barmak Shemirani
Barmak Shemirani

Reputation: 31679

This seems to be a bug in CDialogImpl::OnActivate and CDialogImpl::OnNcActivate:

void CDialogImpl::OnNcActivate(BOOL& bActive)
{
  if (m_Dlg.m_nFlags & WF_STAYACTIVE)
      bActive = TRUE;
  if (!m_Dlg.IsWindowEnabled())
      bActive = FALSE;
}

void CDialogImpl::OnActivate(UINT nState, CWnd* pWndOther)
{
  m_Dlg.m_nFlags &= ~WF_STAYACTIVE;
  CWnd* pWndActive = (nState == WA_INACTIVE) ? pWndOther : &m_Dlg;
  if (pWndActive != NULL)
  {
      BOOL bStayActive = (pWndActive->GetSafeHwnd() == m_Dlg.GetSafeHwnd()
            || pWndActive->SendMessage(WM_FLOATSTATUS, FS_SYNCACTIVE));
      if (bStayActive)
          m_Dlg.m_nFlags |= WF_STAYACTIVE;
  }
  else
  {
      m_Dlg.SendMessage(WM_NCPAINT, 1);
  }
}

This is meant to give CDialogEx the ability to stay active, for example, when CMFCPopupMenu is shown.

But m_Dlg.SendMessage(WM_NCPAINT, 1) is a suspicious call. The usage doesn't match the documentation for WM_NCPAINT:

Parameters

wParam A handle to the update region of the window. The update region is clipped to the window frame.

lParam

This parameter is not used.

Additionally, OnNcActivate has an override based on IsWindowEnabled(). This seems to be a patch to fix the earlier problem in OnActivate. But it causes problems elsewhere, for example when using CFileDialog in CDialogEx

Suggested solution:

Modify CDialogEx::OnActivate so that it runs the default procedure. Or, change it such that it will force repaint.

BOOL CDialogEx::OnNcActivate(BOOL active)
{
    if(m_nFlags & WF_STAYACTIVE)
        active = TRUE;
    return(BOOL)DefWindowProc(WM_NCACTIVATE, active, 0L);
}

void CDialogEx::OnActivate(UINT nState, CWnd* pWndOther, BOOL bMinimized)
{
    Default();
}

or

void CDialogEx::OnActivate(UINT nState, CWnd* pWndOther, BOOL bMinimized)
{
    Default();

    //save the previous flag
    UINT previous_flag = m_nFlags;
    m_nFlags &= ~WF_STAYACTIVE;

    // Determine if this window should be active or not:
    CWnd* pWndActive = (nState == WA_INACTIVE) ? pWndOther : this;
    if(pWndActive != NULL)
    {
        BOOL bStayActive = pWndActive->GetSafeHwnd() == GetSafeHwnd() ||
            pWndActive->SendMessage(WM_FLOATSTATUS, FS_SYNCACTIVE);
        if(bStayActive)
            m_nFlags |= WF_STAYACTIVE;
    }

    if(previous_flag != m_nFlags && previous_flag & WF_STAYACTIVE)
    {
        //if the flag is changed, 
        //and if WF_STAYACTIVE was previously set, 
        //then OnNcActivate had handled it wrongly, do it again
        SendMessage(WM_NCACTIVATE, FALSE); //<- less wrong!
    }
}

This should work with CMFCPopupMenu for example. The MFC menu will open without deactivating the dialog.

I am not sure what SendMessage(WM_FLOATSTATUS, FS_SYNCACTIVE) is for, I haven't been able to test it... If it's necessary, it seems the code could be added on OnNcActivate, and then OnActivate is left alone.

Upvotes: 2

c00000fd
c00000fd

Reputation: 22327

OK, as much as I appreciate @VuVirt's solution, it doesn't completely remove all the bugs that are shipped in the default Dialog-based solution in VS2017. It solves the title bar focus issue, but while continuing to develop my project I encountered another bug. So I'm copy-and-pasting it from my comment to his answer:

There's still some kinda screw-up there. I'm not sure if it's related to this fix or not. Ex: If you create a button and then in its handler try to do: CFileDialog d(TRUE, NULL, NULL, OFN_HIDEREADONLY | OFN_EXPLORER, NULL, this); d.DoModal(); to open a file picker dialog. When file picker opens up, close it and see if the title bar of the parent MFC dialog window goes back to being active. In my case it remains inactive until I click onto the Windows taskbar and then back onto that MFC app.

After banging my head against the wall trying to see what is going on there, I decided to try an earlier solution proposed by @zett42 in the comments to my original question (i.e. to replace CDialogEx with CDialog) and it worked! All the bugs are gone!

So here's my verdict: CDialogEx is buggy af.

The resolution is quite simple: When you create a new dialog-based project use project-wide find-and-replace (in the Edit menu) and replace all occurrences of CDialogEx with CDialog. And that is it. (I tried to use VS2017's refactoring tool for that but it messed it up and didn't replace it all. So simple search-and-replace does the job.)

And if you think that you'll be missing some functionality without CDialogEx, then you won't. All it does (besides introducing bugs) is that it adds background images and colors to the dialog.

So until MS fixes those glaring bugs in their templates I'm sticking with this approach.

Upvotes: 2

VuVirt
VuVirt

Reputation: 1917

I managed to replicate your problem and found a quick fix for it. You need to add the WM_ACTIVATE message handler to your main dialog, comment out the base class OnActivate and modify it like this:

void CMFCApplication1Dlg::OnActivate(UINT nState, CWnd* pWndOther, BOOL bMinimized)
{
    //CDialogEx::OnActivate(nState, pWndOther, bMinimized);

    // TODO: Add your message handler code here
    this->Default();
}

CWnd::Default call is needed to keep the active/inactive visualization of the default button.

Upvotes: 6

Related Questions