[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [lmi] System crashes with invalid input using input sequence GUI
From: |
Vadim Zeitlin |
Subject: |
Re: [lmi] System crashes with invalid input using input sequence GUI |
Date: |
Thu, 21 May 2015 01:31:04 +0200 |
On Wed, 20 May 2015 14:47:33 +0000 "Boutin, Wendy" <address@hidden> wrote:
BW> File | New | Illustration
BW> Payments tab
BW> Select ellipsis for 'Individual payment'
BW> Change 'until maturity' to 'until age'
BW> Select 'Add' button
BW>
BW> There is clearly an input validation error occurring,
BW> but the system crashes when the diagnostic appears.
Hello again,
I did end up by modifying wxWidgets to make sure such crashes do not
happen in the future, see commit[1]. However lmi UI still behaves
surprisingly (at least I personally find the behaviour described in [2]
quite unusual) even with that fix and I think this really needs to be fixed
at lmi level instead, so updating your wxWidgets version is not currently
required -- even though it's arguably always better to use a version in
which this crash can never happen.
BW> It'd be cleaner if it behaved similar to:
BW>
BW> File | New | Illustration
BW> Payments tab Select ellipsis for 'Individual payment'
BW> - Change 'until maturity' to 'until age'
BW> + Change 'until maturity' to 'until retirement'
BW> Select 'Add' button
The trouble is that adding a new row when the first row contains "until
age 99", which is the maximal allowed age here, apparently, results in a
negative range of possible values for the spin control in the new row. This
doesn't happen in the "until retirement" case, as there are still 34 (±1,
as a typical programmer I can never get the bounds right...) years until
the maximal age.
The patch below just disables the spin control if there is no valid range
for it. I am not sure if it's the best solution UI-wise, but I can't think
of anything better right now and it does fix the crash (even without the
updated wxWidgets). A possible alternative would be to disable the "Add"
button if there are no valid values for the controls in a new row, but this
wouldn't could be quite confusing as nothing would indicate to the user
that the age must be reduced in order to allow a new row to be added. So if
you prefer such more pro-active approach, I think it would make sense to at
least show a message like "Sequence cannot be extended due to age 99
running up to the maturity" (but preferably better worded). Please let me
know if you'd like me to make an alternative patch doing it like this.
In the meanwhile, here is the patch I used and testes so far:
---------------------------------- >8 --------------------------------------
diff --git a/input_sequence_entry.cpp b/input_sequence_entry.cpp
index 735f749..d271f39 100644
--- a/input_sequence_entry.cpp
+++ b/input_sequence_entry.cpp
@@ -945,21 +945,28 @@ void InputSequenceEditor::adjust_duration_num_range(int
row)
int const prev_duration = (row > 0) ? duration_scalars_[row - 1] : 0;
wxSpinCtrl& duration = duration_num_field(row);
+ int
+ range_min = -1,
+ range_max = -1;
+
switch(duration_mode_field(row).value())
{
case e_attained_age:
{
- duration.SetRange(input_.issue_age() + 1 + prev_duration,
input_.maturity_age() - 1);
+ range_min = input_.issue_age() + 1 + prev_duration;
+ range_max = input_.maturity_age() - 1;
break;
}
case e_duration:
{
- duration.SetRange(1 + prev_duration, input_.years_to_maturity() -
1);
+ range_min = 1 + prev_duration;
+ range_max = input_.years_to_maturity() - 1;
break;
}
case e_number_of_years:
{
- duration.SetRange(1, input_.years_to_maturity() - prev_duration -
1);
+ range_min = 1;
+ range_max = input_.years_to_maturity() - prev_duration - 1;
break;
}
case e_maturity:
@@ -972,6 +979,15 @@ void InputSequenceEditor::adjust_duration_num_range(int
row)
break;
}
}
+ if(range_min <= range_max)
+ {
+ duration.Enable();
+ duration.SetRange(range_min, range_max);
+ }
+ else
+ {
+ duration.Disable();
+ }
}
void InputSequenceEditor::adjust_duration_num(int row)
---------------------------------- >8 --------------------------------------
Please let me know if there is anything else I can do about this bug,
VZ
[1]
https://github.com/wxWidgets/wxWidgets/commit/5f8ac45789f00467e0f7fc588ac18a3561815cd3
[2]
https://github.com/wxWidgets/wxWidgets/commit/ba107a9a873abf7fefe003ef370e0f3c8c9239aa