[patch] wxNotebook::HitTest() not implemented in wxOSX/Cocoa

[dhyams]

wxWidgets 2.9.1, OSX carbon (perhaps cocoa too, not sure)

Under OSX, theHitTest() routine always returns -1 (not found) no matter what. The routine has been complete disabled with an #if 0 (see src/osx/notebook.cpp). Here is a link to that code:

http://trac.wxwidgets.org/browser/wxWidgets/trunk/src/osx/notebook_osx.cpp#L248

If I just reenable the routine by changing the #if 0 to an #if 1, and remove the WXUNUSED macros, everything works fine again.

So is there a reason HitTest() has been completely removed, or was it an accident? It seems, so far, that it works perfectly well under carbon, so the code should at least stay intact for any carbon build. I can't vouch for cocoa.

[disc]

I will restore the code for Carbon (which only reports wxBK_HITTEST_NOWHERE or wxBK_HITTEST_ONLABEL btw, same as in 2.8). I don't know how to fix this for Cocoa so I'm leaving the ticket open and making it Cocoa specific.

[DS]

(In [67679]) Restored wxNotebook::HitTest for wxOSX-Carbon.

Since the copying of src/osx/carbon/notebmac.cpp to src/osx/notebook_osx.cpp in r55202 the code in wxNotebook::HitTest has been disabled. Enabled it again for at least the Carbon build.

See #13045.

[vadz]

It looks like it would be pretty simple to implement this using  NSTabView::tabViewItemAtPoint.

Any takers?

[dhyams]

I'm trying, but apparently I don't know how things fit together. I wrote a small bit of code that implements HitTest for Cocoa (using NSTabView::tabViewItemAtPoint), and put it in src/osx/cocoa/notebook.mm.

Also effectively commented out HitTest over in src/osx/notebook_osx.cpp by moving the carbon #ifdefs.

Then compiled, install wx, and ran a test app. My HitTest is not being called. I'm just not sure how the pieces are all supposed to fit together...I'm not an objective C programmer, nor Mac programmer, nor wx programmer.

If I post the bit of code that implements HitTest, can someone teach me where to put it so that it will be called? I'm pretty sure that is either correct or close to correct.

[vadz]

Just for the reference, this is also discussed in  this thread.

[dhyams]

OK, this is done. As for the refactoring suggested in the thread referenced one post up, I wasn't really comfortable doing either...best left to the wx pros here I think.

This patch Works For Me (TM); tested and the hit testing is working. I attached the patch onto this ticket.

The patch is against wxWidgets 2.9.4.0, or more accurately, whatever wxWidgets ships with wxPython 2.9.4.0. I think they are the same but...

The only thing unsavory about this patch (full disclosure) is that really, in include/wx/osx/core/private.h, the line added in the patch

+ virtual int TabHitTest( const wxPoint & WXUNUSED(pt), long *flags ) {*flags=1; return -1;};

should really read

+ virtual int TabHitTest( const wxPoint & WXUNUSED(pt), long *flags ) {*flags=wxBK_HITTEST_NOWHERE; return -1;};

But that constant was not defined. I know that the right thing to do is probably to #include "bookctrl.h" in there, but I was a little leery to...I know how fragile the sequence of #includes in a large system can be, so I didn't want to just throw it in there. Maybe the pros can fix this up right. As it stands, 1 is the constant the wxBK_HITTEST_NOWHERE resolves to.

Of course, none of the above is relevant if TabHitTest is pure virtual...but when I tried that, other things broke because they obviously did not implement this virtual function. So to limit the scope of the patch, I provided the default implementation that returns a "miss".

[dhyams]

A word on the "unsavory" part of the patch that should be said...the refactoring suggested by VZ would remove that unsavory code anyway. The reason that it has to be there is that really TabHitTest() as well as a couple of other functions that deal with notebooks shouldn't really be in the class they are in.

Even the default implementation of TabHitTest() should never be called, so the fact that it hardcodes a constant isn't really an issue.

[dhyams]

On this one, is there anything else that I need to do, in order to satisfy everyone that this patch is OK?

[csomor]

apart from the detail that one should use the wxFromNSPoint method for converting, I don't see problems yet, but I will have to test

[SC]

(In [73533]) applying patch, closes #13045