bugGNUstep - Bugs: bug #26950, NSTableView: shift-tab sometimes...

Group
 
 

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

Submitter:  Tim Schmielau <tschmielau>
Submitted:  Fri 03 Jul 2009 04:04:24 PM UTC
   
 
Category:  Gui/AppKit Severity:  3 - Normal
Item Group:  None Status:  Ready For Test
Privacy:  Public Assigned to:  FredKiefer
Open/Closed:  In Test
* Mandatory Fields

Add a New Comment Rich Markup
   

Jump to the original submission

Tue 07 Jul 2009 09:44:22 AM UTC, comment #7: 

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!

Tim Schmielau <tschmielau>
Mon 06 Jul 2009 02:50:10 PM UTC, comment #6: 

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.

Tim Schmielau <tschmielau>
Mon 06 Jul 2009 02:27:03 PM UTC, comment #5: 

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.

Fred Kiefer <FredKiefer>
Group Member
Mon 06 Jul 2009 01:38:24 PM UTC, comment #4: 

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!

Tim Schmielau <tschmielau>
Mon 06 Jul 2009 12:12:36 AM UTC, comment #3: 

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.

Tim Schmielau <tschmielau>
Sun 05 Jul 2009 09:54:10 PM UTC, comment #2: 

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

Fred Kiefer <FredKiefer>
Group Member
Sun 05 Jul 2009 09:50:39 AM UTC, 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...

Fred Kiefer <FredKiefer>
Group Member
Fri 03 Jul 2009 04:04:24 PM UTC, original submission:  

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.

Tim Schmielau <tschmielau>

 

(Note: upload size limit is set to 16384 kB, after insertion of the required escape characters.)

Attach Files:
   
   
Comment:
   

Attached Files
file #18366:  NSTableView.m.patch added by tschmielau (388B - application/octet-stream - patch to fix the problem)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by FredKiefer (Posted a comment)
  • -email is unavailable- added by tschmielau (Submitted the item)
  •  

    There are 0 votes so far. Votes easily highlight which items people would like to see resolved in priority, independently of the priority of the item set by tracker managers.

    Only logged-in users can vote.

     

    Follow 5 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2009-07-05 FredKiefer StatusIn Progress Ready For Test
        Open/ClosedOpen In Test
    2009-07-05 FredKiefer StatusNone In Progress
        Assigned toNone FredKiefer
    2009-07-03 tschmielau Attached File- Added NSTableView.m.patch, #18366

    Back to the top

    Powered by Savane 3.13-d3ae.
    Corresponding source code