#439 TBS_TOOLTIPS not working correctly in debug builds

[Ognyan Chernokozhev]

Setting TBS_TOOLTIPS style to a trackbar in a TDialog displays incorrect tooltips when dragging the thumb bar. For a full explanation and a workaround, check the thread https://sourceforge.net/p/owlnext/discussion/97175/thread/2457c06992/.

Proposed solution: In the TWindow::EvNotify code that handles TTN_NEEDTEXT, set the debug tooltip text only if TTooltipText::szText is empty.

[Vidar Hasfjord]

Hi Jogy, as stated in the discussion, I propose that we remove the modification of the tooltip text and instead generate a diagnostic message in debug builds, hence causing less user confusion. Debugging should not cause changes in program behaviour.

Perhaps using WARNX on a higher trace level is appropriate in this case, as not to flood the diagnostics with these messages by default.

[Vidar Hasfjord]

Proposed code change:

:::C++
  // Intercept requests for tooltip texts and turn the request into
  // a command enabler. This mechanism allows us to route the request
  // the same way commands are routed in OWL. Therefore, the object that
  // handles a command is the object that gets first crack at providing
  // the tip text for that command.
  //
  if (notifyInfo.code == TTN_NEEDTEXT)
  {
    auto& ttText = reinterpret_cast<TTooltipText&>(notifyInfo);
    auto enabler = TTooltipEnabler{ttText, GetHandle()};
    const auto cmdTarget = GetParentO() ? GetParentH() : GetHandle();
    RouteCommandEnable(cmdTarget, enabler);
    const auto handled = enabler.GetHandled();
    WARNX(OwlWin, !handled, 1, _T("No command enabler found for TTN_NEEDTEXT (Id: ") << enabler.GetId() << _T(")."));
    if (handled)
      return 0;
  }

Note that I made some corrections in the code comment as well.

[Vidar Hasfjord]

By the way, consider the logic in the following line:

:::C++
const auto cmdTarget = GetParentO() ? GetParentH() : GetHandle();

It seems this test means to ensure that the command target is encapsulated by OWL. That makes sense, since non-OWL objects do not process WM_COMMAND_ENABLE messages anyway. However, GetParentH returns the parent of the window handle (Windows element), which may not correspond to the parent of the TWindow. So to be more precise, it seems the code should be written as follows:

:::C++
const auto cmdTarget = GetParent() ? GetParent()->GetHandle() : GetHandle();

Which then allows the following simplification:

:::C++
const auto cmdTarget = GetParent() ? GetParent() : this;
RouteCommandEnable(cmdTarget->GetHandle(), enabler);

Does that make sense?

[Ognyan Chernokozhev]

There should not be cases in which the parent TWindow handle differs from the parent window handle as managed by Windows - check for example the code in TWindow::SetParent that should ensure both are in sync.

Still, there are very few places where GetParentH() is used, so in this case it is better not use it, and the code looks better.

[Vidar Hasfjord]

Let me know if you approve and want me to commit the change.

[Ognyan Chernokozhev]

Yes, looks good, go ahead. Will you also make the change in 6.44 branch?

[Vidar Hasfjord]

There should not be cases in which the parent TWindow handle differs from the parent window handle as managed by Windows

I vaguely recall having run into a situation where they did not exacly correlate. If I remember correctly, the handle of the TWindow::Parent was still an ancestor of the window in question, but not the immediate parent. Since then I have been suspicious of window parenthood. :-)

Edit: The code in TWindow::SetParent does seem to force a correlation by calling ::SetParent, so maybe I have been mistaken in my understanding. I've edited this post and removed previous comments as not to create confusion.

Yes, looks good, go ahead. Will you also make the change in 6.44 branch?

Ok! I will merge the fix into 6.44 also.

[Vidar Hasfjord]

This issue was fixed as proposed in [r4553]. The fix was merged into Owlet [r4554] and 6.44 [r4555].

Note that a similar change was made in Owlet in [r2369]. However, that revision generates a OWL_TRACEX message rather than a OWL_WARNX message. The code has now been modified to use the latter, bringing the code in line with the trunk.