Re: [lmi] Anomaly: empty messagebox

[Vadim Zeitlin]

On Sat, 28 Jan 2017 12:28:54 +0000 Greg Chicares <address@hidden> wrote:

GC> To reproduce:
GC>   File | New | Illustration
GC> Clear any input-sequence textcontrol (so that no text remains),
GC> then click its ellipsis button. I see a messagebox with the
GC> textcontrol's name in its titlebar, which contains "OK" and
GC> "Cancel" buttons, but no message.

 Thanks for the instructions, I can indeed reproduce this perfectly well.

GC> I looked in 'input_sequence_entry.cpp', but I can't see how it
GC> could produce this messagebox. Vadim, can you figure this out?

 Yes, it's a bug introduced in my layout optimization patch. The real
problem is that layout_freeze_count_ is never decremented in case of a
premature return from InputSequenceEditor::sequence(). The following patch
fixes it and makes sure it can't happen in the future:
---------------------------------- >8 --------------------------------------
diff --git a/input_sequence_entry.cpp b/input_sequence_entry.cpp
index 7a578a7..ad9bed2 100644
--- a/input_sequence_entry.cpp
+++ b/input_sequence_entry.cpp
@@ -52,6 +52,7 @@
 #include <algorithm>                    // std::copy()
 #include <iterator>                     // std::back_inserter()
 #include <map>
+#include <memory>                       // std::unique_ptr<>
 #include <vector>
 
 namespace
@@ -413,6 +414,14 @@ void InputSequenceEditor::sequence(InputSequence const& s)
 {
     LayoutOnceGuard guard(this);
 
+    // The layout is initially frozen, ensure that it will be thawed now that
+    // we can finally determine our real size.
+    auto thaw_layout = [this](void*) { --layout_freeze_count_; };
+    std::unique_ptr<void, decltype(thaw_layout)> thaw_layout_on_scope_exit
+        (reinterpret_cast<void*>(1) // Never used but must be non-zero.
+        ,thaw_layout
+        );
+
     while(0 < rows_count_)
         {
         remove_row(0);
@@ -485,10 +494,6 @@ void InputSequenceEditor::sequence(InputSequence const& s)
     value_field_ctrl(0).SetFocus();
 
     update_diagnostics();
-
-    // The layout was frozen initially, thaw it now, just once, as we can
-    // determine our really final size.
-    --layout_freeze_count_;
 }
 
 std::string InputSequenceEditor::sequence_string()
---------------------------------- >8 --------------------------------------

I don't know what do you think of the ugly use of unique_ptr<> as poor
C++11 programmer replacement of the well-known "scope guard" class.
Personally I'd prefer to use a specific scope_exit class along the lines of
http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2016/p0052r2.pdf but I
wanted to produce the smallest possible patch for this bug right now in
order to fix the user-visible problem a.s.a.p. If you'd like me to make a
patch with scope_exit-lookalike, I'd be happy to do it. Alternatively we
could, of course, make an ad hoc class just decrementing the freeze count
in its dtor and use it just here.


GC> IOW, is this the intended behavior of the following code path?
GC> 
GC>     if(intervals.empty())
GC>         {
GC>         // have single row (initial state)
GC>         add_row();
GC>         return;
GC>         }
GC> 
GC> If that's the explanation, then maybe the right thing to do
GC> is to establish !intervals.empty() as a ctor postcondition;
GC> I was thinking of doing that anyway.

 I can't really comment on this, I don't know what would be the most
reasonable interpretation of an empty control contents, but I think the
current code works correctly, from the user point of view, once the bug
above, preventing the (created, but invisible) first row from being laid
out correctly, is fixed.

 As usual, sorry for breaking this in the first place,
VZ