From 639a82a4a0d901f37d0adb917d3e18b8e24b1bef Mon Sep 17 00:00:00 2001
From: Henric Jungheim <software@henric.org>
Date: Mon, 29 Jun 2020 08:34:50 -0700
Subject: [PATCH] Be more careful when handling grid keyboard navigation.
 (#589)

If the grid is empty or does not have a selected cell, the current
row and column must still maintain these class invariants:
   -1 <= current_row < rows
   -1 <= current_column < columns
    if either current_row or current_column is -1, then the other
       shall also be -1

wxGrid uses wxGridNoCellCoords to test for current_row == -1 &&
current_column == -1.  We treat the case where only one
of the coordinates is -1 as if both are -1.
---
 src/widgets/Grid.cpp | 78 ++++++++++++++++++++++++++++----------------
 1 file changed, 49 insertions(+), 29 deletions(-)

diff --git a/src/widgets/Grid.cpp b/src/widgets/Grid.cpp
index 05eb6de19..ce20d283d 100644
--- a/src/widgets/Grid.cpp
+++ b/src/widgets/Grid.cpp
@@ -539,29 +539,39 @@ void Grid::OnKeyDown(wxKeyEvent &event)
       {
          int rows = GetNumberRows();
          int cols = GetNumberCols();
-         int crow = GetGridCursorRow();
-         int ccol = GetGridCursorCol();
 
-         if (event.GetKeyCode() == WXK_LEFT) {
-            if (crow == 0 && ccol == 0) {
-               // do nothing
+         const bool has_cells = rows > 0 && cols > 0;
+
+         if (has_cells) {
+            int crow = GetGridCursorRow();
+            int ccol = GetGridCursorCol();
+
+            const bool has_no_selection = crow == wxGridNoCellCoords.GetRow() || ccol == wxGridNoCellCoords.GetCol();
+
+            if (has_no_selection) {
+               SetGridCursor(0, 0);
             }
-            else if (ccol == 0) {
-               SetGridCursor(crow - 1, cols - 1);
+            else if (event.GetKeyCode() == WXK_LEFT) {
+               if (crow == 0 && ccol == 0) {
+                  // do nothing
+               }
+               else if (ccol == 0) {
+                  SetGridCursor(crow - 1, cols - 1);
+               }
+               else {
+                  SetGridCursor(crow, ccol - 1);
+               }
             }
             else {
-               SetGridCursor(crow, ccol - 1);
-            }
-         }
-         else {
-            if (crow == rows - 1 && ccol == cols - 1) {
-               // do nothing
-            }
-            else if (ccol == cols - 1) {
-               SetGridCursor(crow + 1, 0);
-            }
-            else {
-               SetGridCursor(crow, ccol + 1);
+               if (crow == rows - 1 && ccol == cols - 1) {
+                  // do nothing
+               }
+               else if (ccol == cols - 1) {
+                  SetGridCursor(crow + 1, 0);
+               }
+               else {
+                  SetGridCursor(crow, ccol + 1);
+               }
             }
          }
 
@@ -574,11 +584,6 @@ void Grid::OnKeyDown(wxKeyEvent &event)
 
       case WXK_TAB:
       {
-         int rows = GetNumberRows();
-         int cols = GetNumberCols();
-         int crow = GetGridCursorRow();
-         int ccol = GetGridCursorCol();
-
          if (event.ControlDown()) {
             int flags = wxNavigationKeyEvent::FromTab |
                         ( event.ShiftDown() ?
@@ -587,9 +592,17 @@ void Grid::OnKeyDown(wxKeyEvent &event)
             Navigate(flags);
             return;
          }
-         else if (event.ShiftDown()) {
-            // Empty grid?
-            if (crow == -1 && ccol == -1) {
+
+         int rows = GetNumberRows();
+         int cols = GetNumberCols();
+         int crow = GetGridCursorRow();
+         int ccol = GetGridCursorCol();
+
+         const auto is_empty = rows <= 0 || cols <= 0;
+         const auto has_no_selection = crow == wxGridNoCellCoords.GetRow() || ccol == wxGridNoCellCoords.GetCol();
+
+         if (event.ShiftDown()) {
+            if (is_empty) {
                Navigate(wxNavigationKeyEvent::FromTab | wxNavigationKeyEvent::IsBackward);
                return;
             }
@@ -598,6 +611,10 @@ void Grid::OnKeyDown(wxKeyEvent &event)
                Navigate(wxNavigationKeyEvent::FromTab | wxNavigationKeyEvent::IsBackward);
                return;
             }
+
+            if (has_no_selection) {
+               SetGridCursor(rows -1, cols - 1);
+            }
             else if (ccol == 0) {
                SetGridCursor(crow - 1, cols - 1);
             }
@@ -606,8 +623,7 @@ void Grid::OnKeyDown(wxKeyEvent &event)
             }
          }
          else {
-            // Empty grid?
-            if (crow == -1 && ccol == -1) {
+            if (is_empty) {
                Navigate(wxNavigationKeyEvent::FromTab | wxNavigationKeyEvent::IsForward);
                return;
             }
@@ -616,6 +632,10 @@ void Grid::OnKeyDown(wxKeyEvent &event)
                Navigate(wxNavigationKeyEvent::FromTab | wxNavigationKeyEvent::IsForward);
                return;
             }
+
+            if (has_no_selection) {
+               SetGridCursor(0, 0);
+            }
             else if (ccol == cols - 1) {
                SetGridCursor(crow + 1, 0);
             }