Opened 5 years ago

Closed 5 years ago

#10157 closed defect (fixed)

wxOwnerDrawnComboBox SetFocus problem

Reported by: Trigve Owned by:
Priority: normal Milestone:
Component: wxMSW Version: stable-latest
Keywords: wxPopupWindow Vista wxOwnerDrawnComboBox SetFocus Cc:
Blocked By: Blocking:
Patch: yes

Description

When using wxOwnerDrawnComboBox (or wxComboCtrl) derived combo box, I've got a SetFocus error
when clicking to drop-down button:

19:17:25: Debug: ..\..\src\msw\window.cpp(655): 'SetFocus' failed with error 
0x00000057 (The parameter is incorrect.).

stack:

 	wxmsw290ud_core_vc_custom.dll!wxWindow::SetFocus()  Line 655	C++
 	wxmsw290ud_core_vc_custom.dll!wxPanel::SetFocus()  Line 100 + 0x2b 
bytes	C++
 	wxmsw290ud_core_vc_custom.dll!wxPopupTransientWindow::Popup(wxWindow *
winFocus=0x04e36f08)  Line 264 + 0x1e bytes	C++
 	wxmsw290ud_core_vc_custom.dll!wxComboCtrlBase::DoShowPopup(const wxRect 
&
rect={...}, int __formal=2)  Line 2036 + 0x1c bytes	C++
 	wxmsw290ud_core_vc_custom.dll!wxComboCtrl::OnTimerEvent(wxTimerEvent &
__formal={...})  Line 813 + 0x25 bytes	C++

The behaviour can be obtained from samples/widgets example.

Attachments (3)

popup.patch download (1.1 KB) - added by Trigve 5 years ago.
suppress_vista_popup_error.patch download (811 bytes) - added by jmsalli 5 years ago.
show_with_showna.patch download (1.3 KB) - added by jmsalli 5 years ago.

Download all attachments as: .zip

Change History (17)

comment:1 Changed 5 years ago by Trigve

  • Patch set

Ok it looks like that wxPOPUP_WINDOW style hasn't been used with window creation method. This patch solves it.

Changed 5 years ago by Trigve

comment:2 Changed 5 years ago by jmsalli

  • Keywords wxPopupWindow Vista added

I've tested your patch (on Win XP however, on which the SetFocus error does not occur), and after some superficial testing with popup and combo samples, I didn't find any unusual behavior. In other words, addition of WS_POPUP flag did not seem introduce any obviously bad side effects.

However, personally I'm not sure if this is 100% appropriate fix, as IIRC there was some reason WS_POPUP was not used for popup windows. Vadim, any comments?

comment:3 Changed 5 years ago by Trigve

I want to add that without this patch I'm unable to focus on any child of popup window.

comment:4 Changed 5 years ago by jmsalli

All right, I think I understand a bit better why this happens. Thing is, even on other Windows versions, wxPopup(Transient)Window child can't be focused. So, setting the focus basically fails there as well, but no error is reported.

For reasons why WS_POPUP flag was not used in wxPopupWindow, see here:

http://lists.wxwidgets.org/pipermail/wx-dev/2002-June/021435.html

So, more appropriate fix would be not to call SetFocus() on children of wxPopupWindow, or IMHO even better to have wxMSW SetFocus() just fail silently in this particular case (ie. when top-level parent is wxPopupWindow).

I'll look into adding workarounds for this issue in wxComboCtrl and wxOwnerDrawnComboBox.

comment:5 Changed 5 years ago by Trigve

At least in Vista I can set focus to the child windows of popup window. Without this feature I can't properly implement custom wxComboCtrl. Without the patch, wxComboCtrl show popup window but the wxComboPopup implementation looks weird. Also I think no keyboard input is processed. So I think that for proper wxComboCtrl working this should be meaningfully solved.

comment:6 Changed 5 years ago by jmsalli

Did you try calling wxComboCtrl::UseAltPopupWindow()? This function has been added specifically for this sort of situation where ability to focus a popup child is critical. Granted, it would be more optimal to use wxPopupWindow with WS_POPUP instead of wxDialog here, but at least child focus should work.

comment:7 Changed 5 years ago by Trigve

Thanks, with wxComboCtrl::UseAltPopupWindow() it looks like it's working fine. I agree with you that it would be optimal to use wxPopupWindow with WS_POPUP instead of wxDialog . So what should be done as the next?

Changed 5 years ago by jmsalli

comment:8 Changed 5 years ago by jmsalli

Ok, I've added patch with a workaround (it modifies wxPopupTransientWindow::Popup()). Hopefully the SetFocus error message should no longer appear.

Trigve, could you please test it?

Thanks!

comment:9 Changed 5 years ago by vadz

What I'd [still] like to know is how does the standard combobox manage to make this work: it can have focus (so it's a popup window apparently) but doesn't steal the activation from its parent. Maybe we should simply show the popup window using ShowWindow(SW_SHOWNA)? Could the solution be really as simple as this (I don't remember if I tried this back in 2002 but this seems to obvious that I'd like to hope I did...).

comment:10 Changed 5 years ago by jmsalli

Well, MSW combo box popup doesn't seem to have any child control per se, so certainly it could use the same trick wxPopupWindow uses currently. Anyway, we are certainly not the only ones having activation stealing problems with popup windows. For instance, combo box popups in VS2005 property grids _do_ steal activation from their parent.

I tried SW_SHOWNA as you suggested (and also SW_SHOWNOACTIVATE), but the popup still seemed to "activate". Attached is a crude patch of changes I used.

Changed 5 years ago by jmsalli

comment:11 Changed 5 years ago by Trigve

Ok, I've tried both patches.

With the suppress_vista_popup_error.patch SetFocus error message is still appearing.
With the show_with_showna.patch one SetFocus error message no longer appear BUT the child window of poupuw window (in my case the wxListViewCtrl) doesn't have focus and I can't use arrows to move inside the list view for example.

So for now the only working solution is to use UseAltPopupWindow().

thanks

comment:12 follow-up: Changed 5 years ago by jmsalli

Thanks for testing. I guess then the SetFocus() needs to be removed altogether from wxMSW wxPopupTransientWindow::Popup() to suppress the message. It probably does not do any good there anyway. I'm going to apply this change in few days unless someone (Vadim?) opposes.

Also, IMHO optional support for WS_POPUP in wxPopupWindow could be added. Enabled perhaps with a new window style (wxPOPUP_WINDOW_CAN_FOCUS_CHILDREN?).

comment:13 in reply to: ↑ 12 Changed 5 years ago by vadz

Replying to jmsalli:

Thanks for testing. I guess then the SetFocus() needs to be removed altogether from wxMSW wxPopupTransientWindow::Popup() to suppress the message. It probably does not do any good there anyway. I'm going to apply this change in few days unless someone (Vadim?) opposes.

No, no objections from me, thanks.

Also, IMHO optional support for WS_POPUP in wxPopupWindow could be added. Enabled perhaps with a new window style (wxPOPUP_WINDOW_CAN_FOCUS_CHILDREN?).

Unfortunately (a I'd really rather make this work by default/without any flag) this looks like a good idea. I still have a feeling we're missing some trick that the native combobox uses here but I just can't spend any time at all on this right now, sorry...

comment:14 Changed 5 years ago by JMS

  • Resolution set to fixed
  • Status changed from new to closed

(In [56968]) To avoid error message, do not even try to set focus on MSW popup window that doesn't have WS_POPUP style (closes #10157)

Note: See TracTickets for help on using tickets.