#507 Font change in SetupWindow is ignored (regression in 7.0)

[Vidar Hasfjord]

Due to a peculiarity in the TDialog creation sequence (WM_SETFONT is dispatched explicitly in TDialog::DialogFunction), the original implementation of TDialog::EvSetFont disabled default processing until the wfFullyCreated flag is set. Since the flag is not set until SetupWindow has completed, WM_SETFONT is not processed within SetupWindow for dialogs.

However, in OWLNext 7.0, as part of resolving another issue related to the aforementioned peculiar TDialog behaviour and its interaction with the implementation of [feature-requests:#113] "TWindow::SetWindowFont overload for TFont", the wfFullyCreated check was moved from TDialog to TWindow::EvSetFont, causing this issue to affect all windows.

Note that this issue only concerns processing of WM_SETFONT in the window itself. Processing of WM_SETFONT for children is not affected by this issue, since the children all have their SetupWindow completed and wfFullyCreated flag set.

void TMyWindow::SetupWindow()
{
  TWindow::SetupWindow();
  SetWindowFont(...); // Does not work. This window is not wfFullyCreated.
  Child.SetWindowFont(...); // Works, since Child is wfFullyCreated.
}

Workaround

To circumvent the issue, post the WM_SETFONT message instead. This ensures it is handled after SetupWindow has completed and wfFullyCreated is set. Alternatively, for non-dialog windows, you can create a handler for WM_SETFONT that simply does default processing.

See [discussion:c945c74ccd].

[Vidar Hasfjord]

A simple fix is to move the check for wfFullyCreated in TWindow::EvSetFont back to TDialog::EvSetFont. After all, it is there to prevent a crash while explicitly dispatching WM_SETFONT in TDialog::DialogFunction.

See https://sourceforge.net/p/owlnext/discussion/97177/thread/071b2a02/#f2d6

Unfortunately, this solution makes multiple inheritance brittle for derived dialogs, as discussed in the link above. Derived dialogs need to make sure that TDialog::EvSetFont is found before TWindow::EvSetFont in response table lookup. Otherwise there will be a crash. Before 7.0, this was not an issue, since TWindow did not have EV_WM_SETFONT in its response table.

That said, ensuring the correct order of response table lookup for multiple bases is a general issue that (unfortunately) is the user's responsibility to get right. So any crash caused by the new code would just be a manifestation of a problem in the client code, which can be fixed by correcting the order. But, still, such a regression would be regrettable.

However, there may be a more robust solution.

We could replace the explicit lookup in DialogFunction and instead post the message to the dialog window itself. That would postpone the handling of the message by putting it in the message queue, and the message would presumably be processed normally after SetupWindow has completed and wfFullyCreated is set. A problem with this solution is that the message is dispatched later than before, which may cause a problem in client code which has found some way to set the font in SetupWindow, which now would be overridden by the later handling of the posted message.

An even better solution, perhaps, is to make the explicit dispatch more robust so that it does not crash, even if DefaultProcessing is called in the event handler. I like this solution since it encapsulates all of the issues locally, with no changes required elsewhere, nor any new special behaviour that needs to be taken into account.

In particular, to make the dispatch robust, we need to set up and restore TApplication::CurrentEvent properly, and we need to guard against recursive calls (which is the effect of calling TWindow::DefaultProcessing in the event handler):

TDialog::DialogFunction(TMsgId msg, TParam1 param1, TParam2 param2)
{
//...

    // Dispatch WM_SETFONT only for the first, non-sent occurance. Subsequent
    // WM_SETFONTs will be dispatched normally in TWindow.
    //
    case WM_SETFONT:
      {
        CHECK(!IsFlagSet(wfFullyCreated));

        struct TEventGuard
        {
          TCurrentEvent& Current;
          TCurrentEvent Previous;

          TEventGuard(TCurrentEvent& ce, TWindow* win, TMsgId msg, TParam1 param1, TParam2 param2)
            : Current{ce}, Previous{ce}
          {
            Current.Win = win;
            Current.Message = msg;
            Current.Param1 = param1;
            Current.Param2 = param2;
          }

          ~TEventGuard() { Current = Previous; }

          auto IsRecursive()
          {
            // Event handlers may call TWindow::DefaultProcessing, which may cause a recursive call.
            // We assume this is the case if the current event matches the previous.
            //
            return Current.Win == Previous.Win &&
              Current.Message == Previous.Message &&
              Current.Param1 == Previous.Param1 &&
              Current.Param2 == Previous.Param2;
          }
        }
        event{GetCurrentEvent(), this, msg, param1, param2};

        auto wasHandled = TResult{FALSE};
        if (!event.IsRecursive())
        {
          auto eventInfo = TEventInfo{msg};
          if (Find(eventInfo))
          {
            Dispatch(eventInfo, param1, param2);
            wasHandled = TRUE;
          }
        }
        return wasHandled;
      }

//...
}

Both solutions remove the need for special handling in TDialog::EvSetFont, which then can be removed.

Let me know if you have any thoughts!

[Vidar Hasfjord]

This issue was fixed in [r5654], and the fix has been merged into Owlet [r5655].

Revisions: A regression (assertion failure in TDialog::DialogFunction) was fixed in [r5680], and the fix has been merged into Owlet [r5681].

[Vidar Hasfjord]

This fix has now been applied to branches/7 [r6680] as well. The fix was released in OWLNext 7.0.12.