[bug #26950] NSTableView: shift-tab sometimes goes to wrong cell

[Ilguiz Latypov-3]

URL:
  <http://savannah.gnu.org/bugs/?26950>

                 Summary: NSTableView: shift-tab sometimes goes to wrong cell
                 Project: GNUstep
            Submitted by: tschmielau
            Submitted on: Fri 03 Jul 2009 04:04:24 PM GMT
                Category: Gui/AppKit
                Severity: 3 - Normal
              Item Group: None
                  Status: None
                 Privacy: Public
             Assigned to: None
             Open/Closed: Open
         Discussion Lock: Any

    _______________________________________________________

Details:

In an NSTableView, the shift-tab key combination is supposed to go to the
previous cell in the same row, and wrap around only if there is no previous
editable cell in the same row.
However, for cells whose row number is larger than the number of columns, the
actual effect is to allways wrap to the previous row.

The attached patch fixes the problem.
I however do not understand what the if condition that it fixes was intended
for in the first place - I would think that this code should just be executed
unconditionally.



    _______________________________________________________

File Attachments:


-------------------------------------------------------
Date: Fri 03 Jul 2009 04:04:24 PM GMT  Name: NSTableView.m.patch  Size: 388B
 By: tschmielau
patch to fix the problem
<http://savannah.gnu.org/bugs/download.php?file_id=18366>

    _______________________________________________________

Reply to this item at:

  <http://savannah.gnu.org/bugs/?26950>

_______________________________________________
  Message sent via/by Savannah
  http://savannah.gnu.org/



_______________________________________________
Bug-gnustep mailing list
Bug-gnustep@...
http://lists.gnu.org/mailman/listinfo/bug-gnustep

[Ilguiz Latypov-3]

Update of bug #26950 (project gnustep):

                  Status:                    None => In Progress            
             Assigned to:                    None => FredKiefer            

    _______________________________________________________

Follow-up Comment #1:

Committed. Thank you for the patch.

But your question, what is going on there at all, is a vaild one. I will have
to inspect that code a bit more. I would think that we should only need to
test, whether there are any previous columns in the same row and use one of
these then. But if this is wrong the other methods here are also wrong...

    _______________________________________________________

Reply to this item at:

  <http://savannah.gnu.org/bugs/?26950>

_______________________________________________
  Nachricht geschickt von/durch Savannah
  http://savannah.gnu.org/



_______________________________________________
Bug-gnustep mailing list
Bug-gnustep@...
http://lists.gnu.org/mailman/listinfo/bug-gnustep

[Ilguiz Latypov-3]

Update of bug #26950 (project gnustep):

                  Status:             In Progress => Ready For Test        
             Open/Closed:                    Open => In Test                

    _______________________________________________________

Follow-up Comment #2:

I added a hopefully full implementation for the two mehtods doing the tab
move. Please give it a try and a review.

    _______________________________________________________

Reply to this item at:

  <http://savannah.gnu.org/bugs/?26950>

_______________________________________________
  Nachricht geschickt von/durch Savannah
  http://savannah.gnu.org/



_______________________________________________
Bug-gnustep mailing list
Bug-gnustep@...
http://lists.gnu.org/mailman/listinfo/bug-gnustep

[Ilguiz Latypov-3]

Follow-up Comment #3, bug #26950 (project gnustep):

Thanks, Fred. I will install GNUstep from svn tomorrow and test it.
At a first glimpse, I'm surprised that the code has become longer, not
shorter. However, I'll only try to understand your change tomorrow.

    _______________________________________________________

Reply to this item at:

  <http://savannah.gnu.org/bugs/?26950>

_______________________________________________
  Message sent via/by Savannah
  http://savannah.gnu.org/



_______________________________________________
Bug-gnustep mailing list
Bug-gnustep@...
http://lists.gnu.org/mailman/listinfo/bug-gnustep

[Ilguiz Latypov-3]

Follow-up Comment #4, bug #26950 (project gnustep):

Changing the if condition to "if (row > -1)" as you did indeed looks like the
right fix to me, thus my first patch was wrong and just papered over the
problem.

Regarding the further additions to make the tab key cycle through the whole
table view I'm not so sure. It definitely makes things consistent now as e.g.
the return key does the same.
However, it traps the focus inside the table view. With the current state
after your patch, we have two separate worlds (or more if there is more than
one table view). The focus is either in the table view (and can cycle through
it), or in one of the other controls (and can cycle through them). To jump
from one to the other, one needs to use the mouse.
I wonder why the latter is actually the case: there must be some code hidden
somewhere that magically excludes table views from that cycle.

Ideally, I'd be able to cycle through all controls in one cycle, thus at the
end of the table view jump to the next control.
Until bug #26949 is fixed, I however prefer your current code as it seems to
make current behaviour fully consistent and allows my band-aid for that bug to
work.

I've tested current cvs (rev 28381) that includes your patch, and could not
find any issues with it.

Thanks!

    _______________________________________________________

Reply to this item at:

  <http://savannah.gnu.org/bugs/?26950>

_______________________________________________
  Message sent via/by Savannah
  http://savannah.gnu.org/



_______________________________________________
Bug-gnustep mailing list
Bug-gnustep@...
http://lists.gnu.org/mailman/listinfo/bug-gnustep

[Ilguiz Latypov-3]

Follow-up Comment #5, bug #26950 (project gnustep):

Based on you feedback on bug #26949 I changed the code here once more. Please
give it another try, tabbing into a different view should work again now.

    _______________________________________________________

Reply to this item at:

  <http://savannah.gnu.org/bugs/?26950>

_______________________________________________
  Nachricht geschickt von/durch Savannah
  http://savannah.gnu.org/



_______________________________________________
Bug-gnustep mailing list
Bug-gnustep@...
http://lists.gnu.org/mailman/listinfo/bug-gnustep

[Ilguiz Latypov-3]

Follow-up Comment #6, bug #26950 (project gnustep):

Great!

However, we'd now also need to find and disable the code that prevents the
focus to jump into the table view from other controls.
And I'd like to ask for the return key to have it's effect changed in the
same way.

I'll have a look.

    _______________________________________________________

Reply to this item at:

  <http://savannah.gnu.org/bugs/?26950>

_______________________________________________
  Message sent via/by Savannah
  http://savannah.gnu.org/



_______________________________________________
Bug-gnustep mailing list
Bug-gnustep@...
http://lists.gnu.org/mailman/listinfo/bug-gnustep

[Ilguiz Latypov-3]

Follow-up Comment #7, bug #26950 (project gnustep):

ok, here is what I figured so far:

The focus never jumps into a table view because NSTableView does not have the
notion of a selectedCell and thus NSControl's acceptsFirstResponder always
returns NO.

To allow jumping into the table view, it would have to implement
becomeFirstResponder. The code apparently needs to be factored out from the
other methods that allow to select a cell, as becomeFirstResponder is sent in
all those cases.

At this point it becomes too complicated for me as a newbie to GNUstep
internals. So, unless somebody wants to help me with it, I'd ask you to
restore the ifdef'ed out parts of your change to cycle through the whole table
view. While that might not be the ideal solution, it's at least consistent.

Thanks!

    _______________________________________________________

Reply to this item at:

  <http://savannah.gnu.org/bugs/?26950>

_______________________________________________
  Message sent via/by Savannah
  http://savannah.gnu.org/



_______________________________________________
Bug-gnustep mailing list
Bug-gnustep@...
http://lists.gnu.org/mailman/listinfo/bug-gnustep