Opened 20 months ago

Closed 7 months ago

#15008 closed defect (fixed)

wxComboPopup hosted control's scrollbar mouse button events never arrive in wxOSX-Cocoa

Reported by: johnr Owned by: csomor
Priority: normal Milestone:
Component: wxOSX-Cocoa Version: 3.0.0
Keywords: wxMouseEvent wxComboPopup wxStyledTextCtrl Cc: christsc@…, sjm.guzman@…
Blocked By: #13841, #13841 Blocking:
Patch: yes

Description

In wxOSX-Cocoa a scroll bar associated with a control such as a wxListCtrl in wxComboPopup does not get mouse events other than mouse move events.

In

  • (bool) WX_filterSendEvent:(NSEvent *) event

all mouse events >= NSLeftMouseDown and <= NSMouseExited
are sent to the window that currently has capture if there is one and marked as handled by default whether they have been handled or not.

Of note, wxComboPopup will get these events but drops any mouse events not in the "popup->control->ClientArea" in src/common/combocmn.cpp/void wxComboPopupEvtHandler::OnMouseEvent at line 899 with event.Skip(false);
This works in other OS code.

The result is that the scrollbar never gets these events and doesn't work.

The attached patch allows further processing if the mouse event is not handled by the window with mouse capture and allows full scrollbar function for a control in a popupwindow. I don't see any bad effects in my code but perhaps there is some uncommented reason to mark the event as handled by default.

Attachments (4)

nonownedwnd_mm.patch download (566 bytes) - added by johnr 20 months ago.
combocmn_fix.patch download (1.0 KB) - added by johnr 19 months ago.
combo_lview.patch download (2.7 KB) - added by johnr 19 months ago.
stctest.cpp download (905 bytes) - added by arondobos 11 months ago.
Short wxStyledTextCtrl program that consistently crashes on OSX when typing '('

Download all attachments as: .zip

Change History (49)

Changed 20 months ago by johnr

comment:1 Changed 20 months ago by johnr

I think the patch I posted is too specific to my own code to be applied. I need to look at this more closely when time permits.

comment:2 Changed 20 months ago by johnr

The unaltered combo sample gives the appearance that a listview works well. The attached combo_lview.patch modifies the combo sample to use a plain wxComboCtrl to demonstrate the lack of scroll bar response with wxComboCtrl and wxComboPopup hosted wxListView. Note also that the blue highlight colour from the listview mouseover routine never appears. It can also be seen in the next comboctrl hosting a wxTreeCtrl.

Remaining polite I will say that the wxComboPopup window routines are rather labyrinthine.

comment:3 Changed 19 months ago by johnr

I think I found it. The difference is in popup Show(). In wxmsw wxWindow->Show() also Raises the window but in mac it only shows the popup. If we add Raise() as does the sample's wxComboCtrlWithCustomPopupAnim::AnimateShow then scrollbars work, mouseover highlights are in the blue selected colour, the hit-test on a listview popup is accurate and popup mouse position.y is no longer relative to the comboctrl.

At first look, barring glitches, this may also fix #15036

comment:4 Changed 19 months ago by johnr

  • Blocked By 13841 added

I have added combocmn_fix.patch that fixes problems with contained control's inactive scrollbars, contained control selection highlights, inaccurate hit-tests and the failure to close noted in ticket #15036.

These problems can be seen in the combo sample's ComboCtrlListViewWithAnimation control when the patch combo_lview.patch is applied to the combo sample. This disables the call to Raise and ShowWithEffect. There is also an ongotfocus counter that is reset by opening the popup that sends to the debug output if it works (absent in msw).

The fix is trivial involving Raise and Show but ideally we would change wxPopupWindow Show() implementation but I am reluctant to dive in there for fear of upsetting its other usages. Changing defines TRANSIENT_POPUPWIN_IS_PERFECT and POPUPWIN_IS_PERFECT for WXMAC as per WXMSW result in a fix but use a frame or dialog along with a different event handler. That option didn't look that desirable.

Both Raise and Show are necessary otherwise we still get one or more of the problems listed above.
Show() alone

  1. No selection highlight until scrollbar clicked
  2. hit-test is inaccurate until scrollbar clicked
  3. popup doesn't close with clicks on other apps unlesss scrollbar clicked first

Raise() alone

  1. scrollbar works
  2. hit-test is inaccurate and not working outside the parent win boundary
  3. popup closes with clicks on other apps

I have re-opened #13841 because the setfocus via onidle interferes with combopopup button click opening when this patch is applied. See my comment there.

I have tested this fairly extensively and will continue to do so but it would be useful if someone else could try it.

Changed 19 months ago by johnr

Changed 19 months ago by johnr

comment:5 Changed 16 months ago by csomor

  • Owner set to csomor
  • Status changed from new to accepted

really strange - MSW does bring the window to the front and calls SetForegroundWindow and Kevin O. made extra efforts to avoid the popup-window getting focus automatically

I can understand that one wants a non obtrusive popup-bubble not stealing the keyboard focus, but I cannot see why we have the two different behaviors ... we had a ticket for that #4453, so I'd opt for two different calls, perhaps reusing the ShowWithoutActivating name, this would open the way for really bringing the two wxPopupWindow::Show() implementations in line.

comment:6 Changed 16 months ago by vadz

Sorry, I don't really understand what's going on here -- what are the different behaviours?

All I can say is that in theory ideal would be to use ShowWithoutActivating() from wxPopupTransientWindow::Popup() to avoid stealing the activation from the TLW parent but still let the controls inside the popup to handle mouse and keyboard events. But the latter is, of course, more important than the former.

comment:7 Changed 16 months ago by csomor

forgot to link r74090

comment:8 Changed 16 months ago by SC

(In [74091]) make sure mouse moved events are always delivered, see #15008

comment:9 Changed 16 months ago by SC

(In [74092]) deal with Cocoa as we do with Carbon, see #15008

comment:10 Changed 16 months ago by SC

(In [74093]) deal with Cocoa as we do with Carbon, see #15008

comment:11 Changed 16 months ago by SC

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

(In [74098]) using ordinary Show for popup windows as on MSW which activates it as well, I'll rewire ShowWithoutActivating for the other use case, fixes #15008

comment:12 Changed 12 months ago by paulclinger

  • Resolution fixed deleted
  • Status changed from closed to reopened

Stefan, I have to reopen this as this last commit broke auto-complete/userlist functionality in wxSTC. I tested on different versions of OSX (10.7, 10.8) and the effect is exactly the same: the app crashes as soon as it tried to open an auto-complete popup, with this stack trace:

0 0x003a1efd ListBoxImpl::Select(int) + 15 (PlatWX.cpp:1100)
1 0x007ae99b AutoComplete::Show(bool) + 59 (AutoComplete.cxx:120)
2 0x007e2cc7 ScintillaBase::AutoCompleteStart(int, char const*) + 1675 (ScintillaBase.cxx:276)
3 0x007e2e91 ScintillaBase::WndProc(unsigned int, unsigned long, long) + 389 (ScintillaBase.cxx:658)
4 0x0039c91d wxStyledTextCtrl::UserListShow(int, wxString const&) + 75 (buffer.h:117)
5 0x0014eb6b wxLua_wxStyledTextCtrl_UserListShow(lua_State*) + 171

Unfortunately, I just noticed that, but the result is 100% reproducible.

I bisected it to the last commit (http://trac.wxwidgets.org/changeset/74098); I also tested reverting that commit and applying it to the current trunk and it fixes the issue.

comment:13 Changed 12 months ago by paulclinger

The issue is also filed as a new ticket: http://trac.wxwidgets.org/ticket/15508

comment:14 Changed 12 months ago by vadz

  • Milestone changed from 2.9.5 to 3.0

Stefan, should I revert r74098 or do you see a better fix? Something must be done before 3.0, crashing is worse than the original problem.

comment:15 Changed 12 months ago by vadz

Does anybody have a simple way of reproducing it? I.e. a minimal patch to the stctest sample ideally?

From just reading the code, I really don't understand why should r74098 result in a crash...

comment:16 follow-up: Changed 12 months ago by paulclinger

Vadim, I'm not sure why that code is relevant either. The only thing comes to mind is that there may be some sizing event called. Unfortunately I have been unable to simplify the code that crashes to make it easier to test, but given that it's reported to happen in vastly different applications, I think the issue is real.

If this is of any help, I do see a window painted with a gray background and it seems to be of the right size, but there is no content and it crashes with the stack trace I showed earlier. Note that I'm using UserListShow and the other reports seem to be using CallTipShow, but I believe they both use Show method from popupwindow.

What's confusing is that the patch file is "src/osx/carbon/popupwin.cpp", but I'm using cocoa. Why would a file for "carbon" make any difference for my build?

comment:17 in reply to: ↑ 16 Changed 12 months ago by vadz

Replying to paulclinger:

What's confusing is that the patch file is "src/osx/carbon/popupwin.cpp", but I'm using cocoa. Why would a file for "carbon" make any difference for my build?

Cocoa port reuses some files under "carbon" for historical reasons, so this is not a mystery on its own. The rest still is however...

comment:18 Changed 11 months ago by paulclinger

Vadim: just a brief update on this. I tried to figure out what in wxPopupWindow::Show(bool show) method "fixes" the issue. If I have an empty method (just "return true" as the body), there is no crash (but obviously there is no popup either). If I add "ShowWithoutActivating()" call (as in the changeset: http://trac.wxwidgets.org/changeset/74098), then there is a popup and no crash.

comment:19 Changed 11 months ago by vadz

Unfortunately I still don't understand what's going on here and without being able to reproduce and, hence, debug it I just can't do much (other than hoping that Stefan looks at it...)

comment:20 Changed 11 months ago by csomor

  • Status changed from reopened to accepted

I hope I'll find the time to look at it

comment:21 follow-up: Changed 11 months ago by johnr

This is also a problem for me with a custom combo autocomplete routine. Mouse over and mouse scroll events all work properly but when the popup shows then keyboard events that would normally be handled by the textbox component do not work. I have not had time to look into it yet or see if I can work around it.

comment:22 follow-up: Changed 11 months ago by johnr

I also see a crash if I click on another program's window while the popup is shown (and yes the popup shows on top of the other program's window). It crashes in wxWindowBase::GetParent() in window.h where this = NULL.

comment:23 in reply to: ↑ 22 Changed 11 months ago by johnr

Replying to johnr:

It crashes in wxWindowBase::GetParent() in window.h where this = NULL.

That was from wxComboFrameEventHandler::OnIdle(...) that I had copied for a derived combo popup class. The original did not check for a NULL winFocused:

wxWindow* winFocused = ::wxWindow::FindFocus();
if( ... && winFocused->GetParent() != ...

I see this has been fixed in combocmn.cpp revision 73809 with 
if ( !winFocused || (

comment:24 in reply to: ↑ 21 ; follow-up: Changed 11 months ago by johnr

Replying to johnr:

Mouse over and mouse scroll events all work properly but when the popup shows then keyboard events that would normally be handled by the textbox component do not work.

The popup takes focus and hence the key strokes. I can work around this but the textbox used as part of my composite control loses focus and so generates onlostfocus events. I can work around that as well but I lose the highlighting of the textbox's text selection as the user types. I am probably missing something here in my own code but it works fine in MSW.

comment:25 in reply to: ↑ 24 Changed 11 months ago by johnr

Replying to johnr:

The popup takes focus and hence the key strokes...

I went back to using pre revision 74098 ShowPopup via ShowWithoutActivating()

comment:26 Changed 11 months ago by arondobos

  • Keywords wxStyledTextCtrl added

Here's a short test program that crashes consistently on OSX-cocoa when I type '(' into the editor. I have a frame with a wxSTC trying to show a calltip. Works fine on Win32/x64, and linux x64. I've compiled wxWidgets 3.0.0-rc2 with these flags: ./configure --prefix=/Users/adobos/local/wx3-svn --enable-stl=yes --enable-debug=no --enable-shared=no --with-cocoa --with-libjpeg=builtin --with-libpng=builtin --with-regex=builtin --with-libtiff=builtin --with-zlib=builtin --with-expat=builtin

I don't know enough about Cocoa/OSX to help resolve it, but I really hope it can be fixed for 3.0.0 final!

Changed 11 months ago by arondobos

Short wxStyledTextCtrl program that consistently crashes on OSX when typing '('

comment:27 Changed 11 months ago by arondobos

Sorry I left this out: the OSX stack trace is below.


Exception Type: EXC_BAD_ACCESS (SIGSEGV)
Exception Codes: KERN_INVALID_ADDRESS at 0x0000000000000010

VM Regions Near 0x10:
-->

TEXT 0000000103e81000-000000010433d000 [ 4848K] r-x/rwx SM=COW /Users/USER/*/stctest.app/Contents/MacOS/stctest

Application Specific Information:
objc_msgSend() selector name: becomeKeyWindow

Thread 0 Crashed:: Dispatch queue: com.apple.main-thread
0 libobjc.A.dylib 0x00007fff8a1dc250 objc_msgSend + 16
1 com.apple.AppKit 0x00007fff8c7ccf04 -[NSWindow _changeKeyAndMainLimitedOK:] + 719
2 com.apple.AppKit 0x00007fff8c7cc76b -[NSWindow _makeKeyRegardlessOfVisibility] + 104
3 com.apple.AppKit 0x00007fff8c7cc6c5 -[NSWindow makeKeyAndOrderFront:] + 25
4 National Renewable Energy Laboratory 0x0000000103f03d99 wxNonOwnedWindowCocoaImpl::Show(bool) + 195
5 National Renewable Energy Laboratory 0x0000000103e929b3 wxNonOwnedWindow::Show(bool) + 51
6 National Renewable Energy Laboratory 0x0000000104177956 ScintillaBase::CallTipShow(Point, char const*) + 578
7 National Renewable Energy Laboratory 0x0000000104178b01 ScintillaBase::WndProc(unsigned int, unsigned long, long) + 1067
8 National Renewable Energy Laboratory 0x000000010409fe22 wxStyledTextCtrl::CallTipShow(int, wxString const&) + 62
9 National Renewable Energy Laboratory 0x0000000103e85661 MyEditor::OnCharAdded(wxStyledTextEvent&) + 181 (stctest.cpp:21)

comment:28 Changed 10 months ago by arondobos

  • Milestone changed from 3.0.0 to 3.0.1
  • Version changed from 2.9-svn to 3.0.0

comment:29 follow-up: Changed 10 months ago by neurodroid

Is there any way to temporarily work around this issue? It makes my app unusable at this point.

comment:30 Changed 10 months ago by neurodroid

  • Cc christsc@… added

comment:31 Changed 10 months ago by jguzman

  • Cc sjm.guzman@… added

comment:32 in reply to: ↑ 29 Changed 10 months ago by johnr

Replying to neurodroid:

Is there any way to temporarily work around this issue? It makes my app unusable at this point.

Revert http://trac.wxwidgets.org/changeset/74098

comment:33 Changed 10 months ago by johnr

Stefan, perhaps you have better ideas for this ticket but for me using Show rather than ShowWithoutActivating() makes the popupwin take focus which I think causes the problems reported above and for me also when using wxRichtextToolTips from mouseover routines.

For comboctrl popups, the combination of ShowWithoutActivating() as it was pre revision 74098 followed by Raise() as I have in the combocmn_fix.patch above works well at least for me.

comment:34 Changed 10 months ago by csomor

Thanks John, I have this ticket placed prominently in my inbox, I know I have to fix this soon, but I really need some hours to dive into all the issues which lead to the change in the first place, I fear breaking other things otherwise ...

comment:35 Changed 10 months ago by jguzman

I also have this ticket under "high priority" for my application. I would be so happy to have this ticket solved! Thanks!

comment:36 Changed 9 months ago by SC

  • Blocked By

(In #13841) (In [75401]) reverting r74098, applying John's version, see #15008, #15765, #13841 will be backported if no regressions are discovered

comment:37 Changed 8 months ago by johnr

This now works with the exception of comboctrls hosted in modal dialogs. I think it is a problem with the modal loop which causes problems in a few other areas. I note the modal loop still consumes 100% CPU usage when running.

comment:38 follow-up: Changed 8 months ago by vadz

On topic of 100% CPU consumption, see http://review.bakefile.org/r/558 -- perhaps you have any comments/ideas about this?

comment:39 in reply to: ↑ 38 ; follow-up: Changed 7 months ago by johnr

Replying to vadz:

On topic of 100% CPU consumption, see http://review.bakefile.org/r/558 -- perhaps you have any comments/ideas about this?

I have been away for a couple of weeks and see that has been applied in r75779. Unfortunately it made no difference to the comboctrl's popup events when hosted in a modal dialog. Seems similar to the non-responsive html help frame displayed from a modal dialog.

With regard to your question I did some tests with the password dialog on the timeout figure in
return DispatchTimeout( m_isInsideYield ? 0 : 1000 );

Figures are approximate, machine dependent and use 1 core of 4.
timeout cpu%
1000 1.8 to 2
100 2.1 to 2.8
10 4 to 4.2
1 20.5
0 100

The modal loop differs from MSW in that mouse hover events still occur in a parent frame hosting a modal dialog. When I swipe the mouse across a ribbon where I show wxRichToolTips then there is some latency there now when the dialog begins and ends its ShowModal state and several tooltips may show simultaneously instead of not showing at all with a brief mouse over. I suppose the main event loop system continues but is slowed by the 1000 figure. I have adjusted my copy to use 10 to avoid this and accept 4% of cpu time until I see another fix.

comment:40 Changed 7 months ago by johnr

  • Blocked By 13841 removed

comment:41 Changed 7 months ago by johnr

  • Blocked By 13841 added

(In #13841) AFAICS this ticket is fixed.

comment:42 Changed 7 months ago by vadz

  • Blocked By

(In #13841) Thanks for your testing!

comment:43 in reply to: ↑ 39 Changed 7 months ago by johnr

I had a quick look at this today. When the comboctrl/popup is hosted in a model dialog then the NSApp doesn't receive any mouse events from the OS when the mouse is inside the popup boundaries.

comment:44 follow-up: Changed 7 months ago by vadz

  • Milestone 3.0.1 deleted

I am not sure what is/are the remaining problem(s) here and how can they be reproduced exactly, could someone please clarify? Or maybe close this ticket and open a new one as a lot of things have changed apparently since it was opened.

In any case, this doesn't look like something we can fix in time for 3.0.1 unfortunately.

comment:45 in reply to: ↑ 44 Changed 7 months ago by johnr

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

Replying to vadz:

I am not sure what is/are the remaining problem(s) here and how can they be reproduced exactly, could someone please clarify? Or maybe close this ticket and open a new one as a lot of things have changed apparently since it was opened.

The original problem in wxComboCtrl's popup window hosted in a non-modal parent is fixed but remains for modal parents. The latter situation has a different cause and I will open a new ticket for it next time I work with OSX.

AFAIK adverse effects with wxSTC caused by one of the above patches are resolved. I have closed the ticket.

Note: See TracTickets for help on using tickets.