NSTableView not setting -clickedRow, -clickedColumn as it should

  • According to the release notes for 10.7, NSTableView should now support contextual menus at the individual cell level:

    > NSTableView/NSOutlineView - Contextual menu support
    >
    > NSTableView and NSOutlineView now have better contextual menu support. Please see the DragNDropOutlineView demo application for an example of how to properly do contextual menus with a TableView.
    >
    > The key thing to note is that clickedRow and clickedColumn will now both be valid when a contextual menu is popped up. In addition, one can dynamically set the popup menu for a particular cell/column in the delegate method willDisplayCell:. NSTableView handles this by properly overriding menuForEvent:, setting the clickedRow/clickedColumn, and calling [NSCell menuForEvent:inRect:ofView] to find the correct menu. If no menu was returned, the menu for the NSTableView will be used. If one right clicks on a selection of items, all the items are highlighted. One should check to see if clickedRow is one of the selected rows, and if it is then do the menu operation on all the selected rows. The clickedRow and clickedColumn properties will still be valid when the action is sent from the NSMenuItem.
    >

    But when I implement the method in my custom cell, though it's being called correctly,  -clickedRow and -clickedColumn are returning -1. As a result I can't tell which row I right-clicked, so I can't process the menu further. Has this change been revoked in 10.8 (which I'm using at the moment)?

    Code:

    - (NSMenu*)        menuForEvent:(NSEvent *)event inRect:(NSRect)cellFrame ofView:(NSView *)view
    {
    NSMenu* menu = nil;

    if([view respondsToSelector:@selector(delegate)])
    {
      id tvDelegate = [(id)view delegate];

      if([view isKindOfClass:[NSTableView class]] && [tvDelegate respondsToSelector:@selector(contextualMenuForTableView:tableColumn:row:)])
      {
      NSTableView* tv = (NSTableView*) view;
      NSInteger  row, column;

      row = [tv clickedRow];
      column = [tv clickedColumn];

    --Graham
  • On Aug 7, 2012, at 11:55 PM, Graham Cox wrote:

    > According to the release notes for 10.7, NSTableView should now support contextual menus at the individual cell level:
    >
    >> NSTableView/NSOutlineView - Contextual menu support
    >>
    >> NSTableView and NSOutlineView now have better contextual menu support. Please see the DragNDropOutlineView demo application for an example of how to properly do contextual menus with a TableView.
    >>
    >> The key thing to note is that clickedRow and clickedColumn will now both be valid when a contextual menu is popped up. In addition, one can dynamically set the popup menu for a particular cell/column in the delegate method willDisplayCell:. NSTableView handles this by properly overriding menuForEvent:, setting the clickedRow/clickedColumn, and calling [NSCell menuForEvent:inRect:ofView] to find the correct menu. If no menu was returned, the menu for the NSTableView will be used. If one right clicks on a selection of items, all the items are highlighted. One should check to see if clickedRow is one of the selected rows, and if it is then do the menu operation on all the selected rows. The clickedRow and clickedColumn properties will still be valid when the action is sent from the NSMenuItem.
    >>
    >
    >
    > But when I implement the method in my custom cell, though it's being called correctly,  -clickedRow and -clickedColumn are returning -1. As a result I can't tell which row I right-clicked, so I can't process the menu further. Has this change been revoked in 10.8 (which I'm using at the moment)?
    >
    > Code:
    >
    > - (NSMenu*)        menuForEvent:(NSEvent *)event inRect:(NSRect)cellFrame ofView:(NSView *)view
    > {
    > NSMenu* menu = nil;
    >
    > if([view respondsToSelector:@selector(delegate)])
    > {
    > id tvDelegate = [(id)view delegate];
    >
    > if([view isKindOfClass:[NSTableView class]] && [tvDelegate respondsToSelector:@selector(contextualMenuForTableView:tableColumn:row:)])
    > {
    > NSTableView*    tv = (NSTableView*) view;
    > NSInteger        row, column;
    >
    > row = [tv clickedRow];
    > column = [tv clickedColumn];

    I can't speak to why it isn't working as expected but since the "view" argument is the table view you can use rowsInRect:/rowAtPoint: and columnIndexesInRect:/columnAtPoint: to derive the location.

    HTH,

    Keary Suska
    Esoteritech, Inc.
    "Demystifying technology for your home or business"
  • On Aug 7, 2012, at 10:55 PM, Graham Cox <graham.cox...> wrote:

    >
    >
    > According to the release notes for 10.7, NSTableView should now support contextual menus at the individual cell level:
    >
    >> NSTableView/NSOutlineView - Contextual menu support
    >>
    >> NSTableView and NSOutlineView now have better contextual menu support. Please see the DragNDropOutlineView demo application for an example of how to properly do contextual menus with a TableView.
    >>
    >> The key thing to note is that clickedRow and clickedColumn will now both be valid when a contextual menu is popped up. In addition, one can dynamically set the popup menu for a particular cell/column in the delegate method willDisplayCell:. NSTableView handles this by properly overriding menuForEvent:, setting the clickedRow/clickedColumn, and calling [NSCell menuForEvent:inRect:ofView] to find the correct menu. If no menu was returned, the menu for the NSTableView will be used. If one right clicks on a selection of items, all the items are highlighted. One should check to see if clickedRow is one of the selected rows, and if it is then do the menu operation on all the selected rows. The clickedRow and clickedColumn properties will still be valid when the action is sent from the NSMenuItem.
    >>
    >
    >
    > But when I implement the method in my custom cell, though it's being called correctly,  -clickedRow and -clickedColumn are returning -1. As a result I can't tell which row I right-clicked, so I can't process the menu further. Has this change been revoked in 10.8 (which I'm using at the moment)?

    No it hasn't changed, but the clicked row is set *after* you return a menu. That way your menu validation code can use it. I think the DragNDropOutlineView demo shows how to do this.

    corbin

    >
    > Code:
    >
    > - (NSMenu*)        menuForEvent:(NSEvent *)event inRect:(NSRect)cellFrame ofView:(NSView *)view
    > {
    > NSMenu* menu = nil;
    >
    > if([view respondsToSelector:@selector(delegate)])
    > {
    > id tvDelegate = [(id)view delegate];
    >
    > if([view isKindOfClass:[NSTableView class]] && [tvDelegate respondsToSelector:@selector(contextualMenuForTableView:tableColumn:row:)])
    > {
    > NSTableView*    tv = (NSTableView*) view;
    > NSInteger        row, column;
    >
    > row = [tv clickedRow];
    > column = [tv clickedColumn];
    >
    >
    >
    >
    > --Graham
  • On Wed, Aug 8, 2012, at 10:38 AM, Corbin Dunn wrote:
    > No it hasn't changed, but the clicked row is set *after* you return a
    > menu. That way your menu validation code can use it. I think the
    > DragNDropOutlineView demo shows how to do this.

    I've always thought this was an extremely odd design decision, since it
    essentially forces you to use a menu delegate to build your menu rather
    than just handing back the correct NSMenu instance from -menuForEvent:.

    Is there a reason that I shouldn't file a bug asking for clickedRow to
    be set as soon as the row is clicked?

    --Kyle Sluder
  • On Aug 8, 2012, at 11:38 AM, Kyle Sluder <kyle...> wrote:

    > On Wed, Aug 8, 2012, at 10:38 AM, Corbin Dunn wrote:
    >> No it hasn't changed, but the clicked row is set *after* you return a
    >> menu. That way your menu validation code can use it. I think the
    >> DragNDropOutlineView demo shows how to do this.
    >
    > I've always thought this was an extremely odd design decision, since it
    > essentially forces you to use a menu delegate to build your menu rather
    > than just handing back the correct NSMenu instance from -menuForEvent:.
    >
    > Is there a reason that I shouldn't file a bug asking for clickedRow to
    > be set as soon as the row is clicked?

    I just hadn't thought that people would have needed the clicked row earlier; that's about it. Yeah, log a bug; there isn't any reason I can't change it.

    corbin

    >
    > --Kyle Sluder
  • On Wed, Aug 8, 2012, at 01:21 PM, Corbin Dunn wrote:
    >
    > On Aug 8, 2012, at 11:38 AM, Kyle Sluder <kyle...> wrote:
    >> Is there a reason that I shouldn't file a bug asking for clickedRow to
    >> be set as soon as the row is clicked?
    >
    > I just hadn't thought that people would have needed the clicked row
    > earlier; that's about it. Yeah, log a bug; there isn't any reason I can't
    > change it.

    Alright, done: <rdar://problem/12059014> (NSTableView waits until after
    calling -menuForEvent: before setting -clickedRow)

    With a bonus: <rdar://problem/12059058> (NSTableView should have a
    -tableView:menu:forEvent: delegate method).

    Thanks!
    --Kyle
  • On Aug 8, 2012, at 2:05 PM, Kyle Sluder <kyle...> wrote:

    > On Wed, Aug 8, 2012, at 01:21 PM, Corbin Dunn wrote:
    >>
    >> On Aug 8, 2012, at 11:38 AM, Kyle Sluder <kyle...> wrote:
    >>> Is there a reason that I shouldn't file a bug asking for clickedRow to
    >>> be set as soon as the row is clicked?
    >>
    >> I just hadn't thought that people would have needed the clicked row
    >> earlier; that's about it. Yeah, log a bug; there isn't any reason I can't
    >> change it.
    >
    > Alright, done: <rdar://problem/12059014> (NSTableView waits until after
    > calling -menuForEvent: before setting -clickedRow)
    >
    > With a bonus: <rdar://problem/12059058> (NSTableView should have a
    > -tableView:menu:forEvent: delegate method).

    Cool; note that with View Based TableViews these types of things are easier to do, since one can just do normal "view stuff", and easily query for what row they are in (via rowForView:)

    corbin
  • On 09/08/2012, at 3:38 AM, Corbin Dunn <corbind...> wrote:

    > No it hasn't changed, but the clicked row is set *after* you return a menu. That way your menu validation code can use it. I think the DragNDropOutlineView demo shows how to do this.

    OKaaay....

    I can figure it from the cell rect as suggested, so it's not a problem, but I also agree that this seems a very odd behaviour, so good to know it can/will be changed.

    What I'm trying to do is to basically get the table view's delegate (or perhaps it should be data source...?) to return a menu for a given row in response to -menuForEvent: on the view. If it returns nil, the table view's menu will be used. But the current design means it really has to go around the houses: the view punts it to the cell, the cell has to grab the view's delegate and punts it back to the delegate. Though the cell has a -menu property, because of the way that table views use cells (same cell for each row in a given column) this isn't much use, because typically the menu needs to be tuned for the item the row corresponds to. If the table's delegate protocol included a method for returning a menu for a given row, that would be nice, and if implemented by the delegate, the cell could be bypassed.

    -----

    I found another issue while exploring this, which I'll mention just to warn other developers.

    NSCell apparently uses NSCopyObject() to make a copy of itself, and NSTableView copies cells at times, e.g. for hit testing. If you have a custom cell subclass that supports copying, DO NOT use [super copyWithZone:] followed by your custom copy stuff. Since super uses NSCopyObject, any pointer ivars that your custom cell has added has get copied verbatim, (unretained) and then if you set a property on the copy that releases the old pointer, it is now overreleased. Shortly afterwards this will cause an EXC_BAD_ACCESS crash. I thought the modern advice was to avoid NSCopyObject() like the plague for this reason, though I notice that Cocoa still does use it itself (particularly NSImage and NSImageRep).

    --Graham
  • On Wed, Aug 8, 2012, at 02:48 PM, Corbin Dunn wrote:
    > Cool; note that with View Based TableViews these types of things are
    > easier to do, since one can just do normal "view stuff", and easily query
    > for what row they are in (via rowForView:)

    It does make it easier for subviews that want to perform their own
    NSEvent handling to do so, but now I'm having the darndest time figuring
    out how that interacts with the highlight NSTableView draws around
    right-clicked rows. It's supposed to be tied to whether you call
    NSTableView's implementation of -menuForEvent: (which is crazy and I'm
    going to file a Radar on), but I can't get it to show up in a very
    simple demo app with just a plain old NSTableView and a data source.

    --Kyle Sluder
  • On 8/9/12 12:58 AM, Graham Cox wrote:
    > NSCell apparently uses NSCopyObject() to make a copy of itself, and
    > NSTableView copies cells at times, e.g. for hit testing. If you have a custom
    > cell subclass that supports copying, DO NOT use [super copyWithZone:]
    > followed by your custom copy stuff. Since super uses NSCopyObject, any
    > pointer ivars that your custom cell has added has get copied verbatim,
    > (unretained) and then if you set a property on the copy that releases the old
    > pointer, it is now overreleased. Shortly afterwards this will cause an
    > EXC_BAD_ACCESS crash. I thought the modern advice was to avoid NSCopyObject()
    > like the plague for this reason, though I notice that Cocoa still does use it
    > itself (particularly NSImage and NSImageRep).

    Not calling super sounds like a bad idea. If you set the ivars of your copy in
    -copyWithZone: that should really not be a problem, it doesn't matter if super
    set them or not. The unretained references will get replaced by whatever you
    choose to do with the ivar in your copy (copy, retain).

    It works fine in a number of custom cells I use (used in outline views and tables).

    Regards
    Markus
    --
    __________________________________________
    Markus Spoettl
  • On 8/9/12 8:14 AM, Markus Spoettl wrote:
    > On 8/9/12 12:58 AM, Graham Cox wrote:
    >> NSCell apparently uses NSCopyObject() to make a copy of itself, and
    >> NSTableView copies cells at times, e.g. for hit testing. If you have a custom
    >> cell subclass that supports copying, DO NOT use [super copyWithZone:]
    >> followed by your custom copy stuff. Since super uses NSCopyObject, any
    >> pointer ivars that your custom cell has added has get copied verbatim,
    >> (unretained) and then if you set a property on the copy that releases the old
    >> pointer, it is now overreleased. Shortly afterwards this will cause an
    >> EXC_BAD_ACCESS crash. I thought the modern advice was to avoid NSCopyObject()
    >> like the plague for this reason, though I notice that Cocoa still does use it
    >> itself (particularly NSImage and NSImageRep).
    >
    > Not calling super sounds like a bad idea. If you set the ivars of your copy in
    > -copyWithZone: that should really not be a problem, it doesn't matter if super
    > set them or not. The unretained references will get replaced by whatever you
    > choose to do with the ivar in your copy (copy, retain).
    >
    > It works fine in a number of custom cells I use (used in outline views and tables).

    Reading the other thread, what I don't do is initialize object ivars in init, so
    that's why this approach works for me.

    Regards
    Markus
    --
    __________________________________________
    Markus Spoettl
  • On 09/08/2012, at 4:14 PM, Markus Spoettl <ms_lists...> wrote:

    > Not calling super sounds like a bad idea.

    Yep, I realise that now.

    So call super, but you need to know whether it's going to call through your -init method or not (for cells, it does not).

    --Graham
  • On Aug 9, 2012, at 00:04 , Graham Cox <graham.cox...> wrote:

    > So call super, but you need to know whether it's going to call through your -init method or not (for cells, it does not).

    Except that you do sort-of know (I think). If it does, all your instance variables are 0. If you have object-pointer instance variables that aren't nil after calling super, then (I think) you can assume that a superclass used NSCopyObject.

    But thinking about Greg's comments earlier in the day, I have started to believe that NSCopyObject isn't in itself a problem. I think the problem is really that you don't know whether the ARC compilation mode of the superclass is the same as the ARC compilation mode of your subclass.

    There's a workaround (or no need of a workaround) for every combination of super/subclass mode. It's that you don't know which workaround to use (or not use) unless you know whether ARC was on or off when the superclass was compiled.

    What do you think?
  • On 09/08/2012, at 5:31 PM, Quincey Morris <quinceymorris...> wrote:

    >> So call super, but you need to know whether it's going to call through your -init method or not (for cells, it does not).
    >
    > Except that you do sort-of know (I think). If it does, all your instance variables are 0. If you have object-pointer instance variables that aren't nil after calling super, then (I think) you can assume that a superclass used NSCopyObject.

    Not necessarily. For cells that might be OK since mostly a cell subclass's -init methods are probably fairly simple, but suppose your subclass has a mutable array for example, and you assign it to be an empty array in -init. Your copy method can't tell by looking at that ivar whether it was copied or created new... I suppose it could check whether the pointer value was equal in the copy, knowing that the -init method assigns a new instance... but while that avoids the flag hack, it's still pretty stinky.

    > But thinking about Greg's comments earlier in the day, I have started to believe that NSCopyObject isn't in itself a problem. I think the problem is really that you don't know whether the ARC compilation mode of the superclass is the same as the ARC compilation mode of your subclass.
    >
    > There's a workaround (or no need of a workaround) for every combination of super/subclass mode. It's that you don't know which workaround to use (or not use) unless you know whether ARC was on or off when the superclass was compiled.

    Yes, I think you've put your finger on it.

    For the case of NSCell, I now know how it currently behaves and can take that into account for copy methods. It's documented, albeit obscurely, and as Greg says, it's probably going to tie Apple's hands on binary compatibility unless some wholesale change is made to the API for copying. (As the Obj-C language gradually gets extended, maybe we could even get something like C++ copy constructors, only hopefully better).

    But trying to generalise the NSCell situation to any subclassable Cocoa class, where you just don't know whether super will use NSCopyObject() with or without ARC enhancements, or alloc/init, it's looking pretty difficult to come up with a general rule about how to correctly implement copy. But perhaps the situation isn't that bad - I haven't run into this very widely, and as NSCopyObject is now deprecated it may only remain an issue for very old classes such as NSCell, and it can be documented as a special case.

    --Graham
  • On Wed, Aug 8, 2012, at 04:37 PM, Kyle Sluder wrote:
    > It does make it easier for subviews that want to perform their own
    > NSEvent handling to do so, but now I'm having the darndest time figuring
    > out how that interacts with the highlight NSTableView draws around
    > right-clicked rows. It's supposed to be tied to whether you call
    > NSTableView's implementation of -menuForEvent: (which is crazy and I'm
    > going to file a Radar on), but I can't get it to show up in a very
    > simple demo app with just a plain old NSTableView and a data source.

    Alright, I've figured out why I wasn't getting the right-click
    highlighting, even when using NSTableView's default implementation: I'd
    accidentally hooked up my menu to the scroll view's menu outlet, rather
    than to the table view's.

    So now we're back to a situation that makes a lot more sense, given what
    we've discussed in this thread. If you override -menuForEvent: and don't
    call super, you don't get the right values for -clickedRow: and
    -clickedColumn:, and you don't get the right-click menu highlighting.

    As I mentioned, I already filed the first; the second is now filed as
    <rdar://problem/12067701> (NSTableView won't show right-click highlight
    if -menuForEvent doesn't call super).

    Thanks to Corbin for listening, and to Graham for motivating me to
    figure out just what the heck is going on in NSTableView so I can fix
    the code I happen to be working on at the moment. :)

    --Kyle Sluder
previous month august 2012 next month
MTWTFSS
    1 2 3 4 5
6 7 8 9 10 11 12
13 14 15 16 17 18 19
20 21 22 23 24 25 26
27 28 29 30 31    
Go to today