From 811be7ced7f272a6fbb5f4dc72ebe8979d406f56 Mon Sep 17 00:00:00 2001
From: Vadim Zeitlin <vadim@wxwidgets.org>
Date: Fri, 29 Nov 2019 19:37:22 +0100
Subject: [PATCH] Fix wxGridCellAttr::GetNonDefaultAlignment() for invalid
 inputs

The recent change of 19844d27acf8177b721e77d3ee696286ae6567ab fixed this
function for valid values of input/output parameters on input, but broke
it if the parameters had invalid value: in this case, we still need to
fill them even if this attribute doesn't have any specific alignment of
its own.

Account for this case too now, explain the logic of this function in the
comments inside it and extend the unit test to check for this case too.

Now the function should really conform to its documented (and expected,
including by the existing code in various grid renderers) behaviour.

Closes https://github.com/wxWidgets/wxWidgets/pull/1665
---
 src/generic/grid.cpp        | 40 +++++++++++++++++++++++++++++--------
 tests/controls/gridtest.cpp | 24 +++++++++++++++++++---
 2 files changed, 53 insertions(+), 11 deletions(-)

diff --git a/src/generic/grid.cpp b/src/generic/grid.cpp
index d4983e04e0..496c9265fd 100644
--- a/src/generic/grid.cpp
+++ b/src/generic/grid.cpp
@@ -582,16 +582,40 @@ void wxGridCellAttr::GetAlignment(int *hAlign, int *vAlign) const
 
 void wxGridCellAttr::GetNonDefaultAlignment(int *hAlign, int *vAlign) const
 {
-    // Default attribute can only have default alignment, so don't return it
-    // from this function.
-    if ( this == m_defGridAttr )
-        return;
+    // The logic here is tricky but necessary to handle all the cases: if we
+    // have non-default alignment on input, we should only override it if this
+    // attribute specifies a non-default alignment. However if the input
+    // alignment is invalid, we need to always initialize it, using the default
+    // attribute if necessary.
 
-    if ( hAlign && m_hAlign != wxALIGN_INVALID )
-        *hAlign = m_hAlign;
+    // First of all, never dereference null pointer.
+    if ( hAlign )
+    {
+        if ( this != m_defGridAttr && m_hAlign != wxALIGN_INVALID )
+        {
+            // This attribute has its own alignment, which should always
+            // override the input alignment value.
+            *hAlign = m_hAlign;
+        }
+        else if ( *hAlign == wxALIGN_INVALID )
+        {
+            // No input alignment specified, fill it with the default alignment
+            // (note that we know that this attribute itself doesn't have any
+            // specific alignment or is the same as the default one anyhow in
+            // in this "else" branch, so we don't need to check m_hAlign here).
+            *hAlign = m_defGridAttr->m_hAlign;
+        }
+        //else: Input alignment is valid but ours one isn't, nothing to do.
+    }
 
-    if ( vAlign && m_vAlign != wxALIGN_INVALID )
-        *vAlign = m_vAlign;
+    // This is exactly the same logic as above.
+    if ( vAlign )
+    {
+        if ( this != m_defGridAttr && m_vAlign != wxALIGN_INVALID )
+            *vAlign = m_vAlign;
+        else if ( *vAlign == wxALIGN_INVALID )
+            *vAlign = m_defGridAttr->m_vAlign;
+    }
 }
 
 void wxGridCellAttr::GetSize( int *num_rows, int *num_cols ) const
diff --git a/tests/controls/gridtest.cpp b/tests/controls/gridtest.cpp
index 722e34e913..f38c0c48bf 100644
--- a/tests/controls/gridtest.cpp
+++ b/tests/controls/gridtest.cpp
@@ -835,13 +835,31 @@ void GridTestCase::GetNonDefaultAlignment()
     // preferred alignment, so check that if we don't reset the alignment
     // explicitly, it doesn't override the alignment used by default.
     wxGridCellAttr* attr = NULL;
-    int align = wxALIGN_RIGHT;
+    int hAlign = wxALIGN_RIGHT,
+        vAlign = wxALIGN_INVALID;
 
     attr = m_grid->CallGetCellAttr(0, 0);
     REQUIRE( attr );
 
-    attr->GetNonDefaultAlignment(&align, NULL);
-    CHECK( align == wxALIGN_RIGHT );
+    // Check that the specified alignment is preserved, while the unspecified
+    // component is filled with the default value (which is "top" by default).
+    attr->GetNonDefaultAlignment(&hAlign, &vAlign);
+    CHECK( hAlign == wxALIGN_RIGHT );
+    CHECK( vAlign == wxALIGN_TOP );
+
+    // Now change the defaults and check that the unspecified alignment
+    // component is filled with the new default.
+    m_grid->SetDefaultCellAlignment(wxALIGN_CENTRE_HORIZONTAL,
+                                    wxALIGN_CENTRE_VERTICAL);
+
+    vAlign = wxALIGN_INVALID;
+
+    attr = m_grid->CallGetCellAttr(0, 0);
+    REQUIRE( attr );
+
+    attr->GetNonDefaultAlignment(&hAlign, &vAlign);
+    CHECK( hAlign == wxALIGN_RIGHT );
+    CHECK( vAlign == wxALIGN_CENTRE_VERTICAL );
 }
 
 void GridTestCase::Editable()