From 78b98bf00c8158f730a0360ca2873e1fa5634193 Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Fri, 3 Apr 2015 16:32:32 +0200 Subject: [PATCH] Validate wxBoxSizer flags instead of silently ignoring invalid combinations. Detect using flags corresponding to the major sizer direction (which doesn't make sense as only the proportion governs the behaviour in this direction) and also combinations of alignment flags with wxEXPAND. --- docs/changes.txt | 2 ++ include/wx/sizer.h | 3 ++ src/common/sizer.cpp | 61 +++++++++++++++++++++++++++++++++++++++ tests/sizers/boxsizer.cpp | 57 ++++++++++++++++++++++++++++++++++-- 4 files changed, 121 insertions(+), 2 deletions(-) diff --git a/docs/changes.txt b/docs/changes.txt index 85541a9e5f..151852f448 100644 --- a/docs/changes.txt +++ b/docs/changes.txt @@ -24,6 +24,8 @@ Changes in behaviour not resulting in compilation errors - Creating wxBitmap with 0 width or height now always fails in all ports (it used to succeed in wxMSW). +- Using invalid flags with wxBoxSizer items now triggers asserts. + Changes in behaviour which may result in build errors ----------------------------------------------------- diff --git a/include/wx/sizer.h b/include/wx/sizer.h index 8e7272ab2a..f2c560d788 100644 --- a/include/wx/sizer.h +++ b/include/wx/sizer.h @@ -941,6 +941,9 @@ public: virtual void RecalcSizes() wxOVERRIDE; protected: + // Only overridden to perform extra debugging checks. + virtual wxSizerItem *DoInsert(size_t index, wxSizerItem *item) wxOVERRIDE; + // helpers for our code: this returns the component of the given wxSize in // the direction of the sizer and in the other direction, respectively int GetSizeInMajorDir(const wxSize& sz) const diff --git a/src/common/sizer.cpp b/src/common/sizer.cpp index ccb4f06eb4..21d6eee3ab 100644 --- a/src/common/sizer.cpp +++ b/src/common/sizer.cpp @@ -2009,6 +2009,67 @@ void wxFlexGridSizer::RemoveGrowableRow( size_t idx ) // wxBoxSizer //--------------------------------------------------------------------------- +wxSizerItem *wxBoxSizer::DoInsert(size_t index, wxSizerItem *item) +{ + const int flags = item->GetFlag(); + if ( IsVertical() ) + { + wxASSERT_MSG + ( + !(flags & wxALIGN_BOTTOM), + wxS("Vertical alignment flags are ignored in vertical sizers") + ); + + // We need to accept wxALIGN_CENTRE_VERTICAL when it is combined with + // wxALIGN_CENTRE_HORIZONTAL because this is known as wxALIGN_CENTRE + // and we accept it historically in wxSizer API. + if ( !(flags & wxALIGN_CENTRE_HORIZONTAL) ) + { + wxASSERT_MSG + ( + !(flags & wxALIGN_CENTRE_VERTICAL), + wxS("Vertical alignment flags are ignored in vertical sizers") + ); + } + + if ( flags & wxEXPAND ) + { + wxASSERT_MSG + ( + !(flags & (wxALIGN_RIGHT | wxALIGN_CENTRE_HORIZONTAL)), + wxS("Horizontal alignment flags are ignored with wxEXPAND") + ); + } + } + else // horizontal + { + wxASSERT_MSG + ( + !(flags & wxALIGN_RIGHT), + wxS("Horizontal alignment flags are ignored in horizontal sizers") + ); + + if ( !(flags & wxALIGN_CENTRE_VERTICAL) ) + { + wxASSERT_MSG + ( + !(flags & wxALIGN_CENTRE_HORIZONTAL), + wxS("Horizontal alignment flags are ignored in horizontal sizers") + ); + } + + if ( flags & wxEXPAND ) + { + wxASSERT_MSG( + !(flags & (wxALIGN_BOTTOM | wxALIGN_CENTRE_VERTICAL)), + wxS("Vertical alignment flags are ignored with wxEXPAND") + ); + } + } + + return wxSizer::DoInsert(index, item); +} + wxSizerItem *wxBoxSizer::AddSpacer(int size) { return IsVertical() ? Add(0, size) : Add(size, 0); diff --git a/tests/sizers/boxsizer.cpp b/tests/sizers/boxsizer.cpp index 75534d2c96..3132c5f061 100644 --- a/tests/sizers/boxsizer.cpp +++ b/tests/sizers/boxsizer.cpp @@ -381,7 +381,60 @@ void BoxSizerTestCase::RecalcSizesRespectsMaxSize2() void BoxSizerTestCase::IncompatibleFlags() { - WX_ASSERT_FAILS_WITH_ASSERT( - m_sizer->Add(10, 10, 0, wxALIGN_BOTTOM | wxALIGN_CENTRE_VERTICAL) +#define ASSERT_SIZER_INVALID_FLAGS(f, msg) \ + WX_ASSERT_FAILS_WITH_ASSERT_MESSAGE( \ + "Expected assertion not generated for " msg, \ + m_sizer->Add(10, 10, 0, f) \ + ) + +#define ASSERT_SIZER_INCOMPATIBLE_FLAGS(f1, f2) \ + ASSERT_SIZER_INVALID_FLAGS(f1 | f2, \ + "using incompatible flags " #f1 " and " #f2 \ + ) + + // In horizontal sizers alignment is only used in vertical direction. + ASSERT_SIZER_INVALID_FLAGS( + wxALIGN_RIGHT, + "using wxALIGN_RIGHT in a horizontal sizer" ); + + ASSERT_SIZER_INVALID_FLAGS( + wxALIGN_CENTRE_HORIZONTAL, + "using wxALIGN_CENTRE_HORIZONTAL in a horizontal sizer" + ); + + // However using wxALIGN_CENTRE_HORIZONTAL together with + // wxALIGN_CENTRE_VERTICAL as done by wxSizerFlags::Centre() should work. + m_sizer->Add(10, 10, wxSizerFlags().Centre()); + + // Combining two vertical alignment flags doesn't make sense. + ASSERT_SIZER_INCOMPATIBLE_FLAGS(wxALIGN_BOTTOM, wxALIGN_CENTRE_VERTICAL); + + // Combining wxEXPAND with vertical alignment doesn't make sense neither. + ASSERT_SIZER_INCOMPATIBLE_FLAGS(wxEXPAND, wxALIGN_CENTRE_VERTICAL); + ASSERT_SIZER_INCOMPATIBLE_FLAGS(wxEXPAND, wxALIGN_BOTTOM); + + + // And now exactly the same thing in the other direction. + delete m_sizer; + m_sizer = new wxBoxSizer(wxVERTICAL); + + ASSERT_SIZER_INVALID_FLAGS( + wxALIGN_BOTTOM, + "using wxALIGN_BOTTOM in a vertical sizer" + ); + + ASSERT_SIZER_INVALID_FLAGS( + wxALIGN_CENTRE_VERTICAL, + "using wxALIGN_CENTRE_VERTICAL in a vertical sizer" + ); + + m_sizer->Add(10, 10, wxSizerFlags().Centre()); + + ASSERT_SIZER_INCOMPATIBLE_FLAGS(wxALIGN_RIGHT, wxALIGN_CENTRE_HORIZONTAL); + ASSERT_SIZER_INCOMPATIBLE_FLAGS(wxEXPAND, wxALIGN_CENTRE_HORIZONTAL); + ASSERT_SIZER_INCOMPATIBLE_FLAGS(wxEXPAND, wxALIGN_RIGHT); + +#undef ASSERT_SIZER_INCOMPATIBLE_FLAGS +#undef ASSERT_SIZER_INVALID_FLAGS }