Re: [lmi] [lmi-commits] master 9bed27ea 1/2: Revert "Remove unnecessary variable from IllustrationView::OnCreate()"

[Greg Chicares]

On 6/15/22 19:18, Vadim Zeitlin wrote:
> On Wed, 15 Jun 2022 14:33:56 -0400 (EDT) Greg Chicares 
> <gchicares@sbcglobal.net> wrote:
[...]
> GC>     Reproducible test case:
> GC>       File | New | Illustration
> GC>       delete contents of "Current COI multiplier" [on "Group" tab]
> GC>       OK
> 
>  Sorry about introducing this problem.
> 
>  After testing it, do I understand correctly that the bug is (only) the
> fact that an unwanted question about saving the changes is asked?

We hope to do our first 64-bit production release this month, and
there are only eight days until our normal code-freeze date, so
I pushed this fix without studying all the history first, or
taking the time to write a really informative commit message.

It didn't seem to matter how the unwanted question was answered:
the dialog closed abruptly for all three answers. With msw
(though perhaps not with GTK), the file can be saved, and later
reopened, but if it's reopened, no view is created, and thus
there's no open document whose input the user can edit to fix.
Of course, if they remember the error message, they can re-reopen
it and fix it then, but they might not be that ingenious. IOW,
the problem isn't only that a stray question is asked, but that
the view isn't created. That makes sense to us, but to an end user
it may not.

The same kind of thing was happening for other detectably invalid
input--for example:

  File | New | Illustration
  "Group" tab:
    make sure "Retirees eligible to enroll" is unchecked
  "Client" tab:
    make sure "Retirement age" is 65 [the default]
    change "Issue age" to 85
  OK

# 0 0x7f7e23682d66: /opt/lmi/src/lmi/unwind.cpp:197 __cxa_throw
# 1 0x7f7e234c7f4c: /opt/lmi/src/lmi/ihs_basicval.cpp:235 BasicValues::Init() 
[clone .cold]
# 2 0x7f7e23702427: /opt/lmi/src/lmi/ihs_basicval.cpp:88 
BasicValues::BasicValues(Input const&)
# 3 0x7f7e236df1e1: /opt/lmi/src/lmi/ihs_acctval.cpp:107 
AccountValue::AccountValue(Input const&)
# 4 0x7f7e236331e9: /opt/lmi/src/lmi/ledgervalues.cpp:40 IllusVal::run(Input 
const&)
# 5 0x7f7e2356334d: /opt/lmi/src/lmi/illustrator.cpp:122 
illustrator::operator()(fs::path const&, Input const&)
# 6 0x7f7e2398bf02: /opt/lmi/src/lmi/illustration_view.cpp:289 
IllustrationView::Run(Input*)
# 7 0x7f7e2398d70f: /opt/lmi/src/lmi/illustration_view.cpp:167 
IllustrationView::OnCreate(wxDocument*, long)
# 8 0x7f7e22d19b7b: ???:0 wxDocTemplate::CreateView(wxDocument*, long)
# 9 0x7f7e22d1963b: ???:0 wxDocument::OnCreate(wxString const&, long)
...

When you have time, could you add a case like that to the GUI test?

>  There is a simple fix for this: we should ignore the view's modified
> status when destroying it unconditionally, and I'll make this change in wx,
> but I'd understand if you didn't want to re-revert this again...

With an automated GUI test for this, we could reconsider later
after testing the behavior with that wx change. But the
behavior after this reversion is that a view is created, and
as explained above I think that's the least astonishing thing
we can do.