Re: [lmi] Input-sequence editor testing

[Vadim Zeitlin]

On Sun, 10 Jul 2011 21:21:31 +0000 Greg Chicares <address@hidden> wrote:

GC> On 2011-07-10 12:44Z, Vadim Zeitlin wrote:
GC> [...Enter works like Tab in dialogs with no enabled default pushbutton...]
GC> >  Anyhow, I'll look at this and will fix it in wxMSW if it doesn't require
GC> > too much time/effort (but I don't think so) and if there are no objections
GC> > from Greg.
GC> 
GC> Yes, it's best to fix it in wxMSW, so of course I wouldn't object to your
GC> spending time on that.

 It didn't take much time to fix it in wxMSW, I did this in r68227 and, as
you can see at http://trac.wxwidgets.org/changeset/68227 it was trivial
(the changes to the sample don't count, only the one in src/msw/window.cpp
does).

GC> Would it also be possible to fix it locally in lmi, so that we can keep
GC> using the official wx-2.9.2 release while preventing the unwanted behavior
GC> in our planned lmi-20110831 release? Presumably I could figure out how to
GC> do it myself using the techniques you described here:
GC>   http://lists.nongnu.org/archive/html/lmi/2011-07/msg00014.html
GC> but it'd take you far less time, so could I ask you to propose a patch?

 I'm very glad you didn't decide to do it yourself because I just spent
more than an hour on this and actually concluded that I couldn't do it in
the way I recommended :-(

 There are actually several more bugs in wx related to this because using
wxTE_PROCESS_ENTER basically disables the normal keyboard navigation and
hence, for example, TAB stops working as well (even without
wxTE_PROCESS_TAB). This can be solved easily for wxTextCtrl for which it's
enough to also intercept wxEVT_KEY_DOWN event and implement navigation in
its handler. This can also be solved for wxComboBox, albeit with more
difficulty, because of another wx bug which requires us to also intercept
wxEVT_CHAR for it. But it doesn't work with wxComboBox using
auto-completion that happens to be the case of the combobox in the sequence
editor (keywords can be auto-completed). And it doesn't work with
wxSpinCtrl neither as while it does work as expected, it also generates a
nasty beep because the TAB key escapes to the default implementation of the
text control in Windows. I could fix this by changing wx sources but this,
of course, doesn't make much sense as the problem we're trying to solve by
using wxTE_PROCESS_ENTER doesn't exist any more in the latest sources
anyhow.

 So finally I solved the problem in a completely different way, by
overriding internal MSWProcessMessage() virtual method for the dialog and
avoiding wxWidgets keyboard navigation code entirely and just using the
native MSW function directly. While this native function is broken in many
various and interesting ways, it does seem to work in this particular,
relatively simple case, although it does require another nested workaround
for another small wxMSW bug (which is never noticed in normal operation but
got uncovered by this hack).

 The only problem with the patch below is that it doesn't keep the same TAB
order as the current version but uses the same order as the original code,
i.e. the "Remove" and "Add" buttons appear in the middle and not the end.
Unfortunately I don't see any easy way to fix this so I can only hope that
it's acceptable for now -- and, of course, the workaround won't be active
any more for wx 2.9.3 and later so the TAB order will be correct again in
them.

 All this being said, here is the patch, please let me know if you have any
further questions or comments about it.

 Thanks,
VZ

diff --git a/input_sequence_entry.cpp b/input_sequence_entry.cpp
index 162ed2b..88706bd 100755
--- a/input_sequence_entry.cpp
+++ b/input_sequence_entry.cpp
@@ -54,6 +54,17 @@
 #include <map>
 #include <vector>
 
+// In wxWidgets up to and including 2.9.2 the "Return" key erroneously worked
+// as TAB in a dialog when the default button was disabled. Work around this
+// problem by completely avoiding wxWidgets keyboard navigation for the input
+// sequence entry dialog by clearing wxTAB_TRAVERSAL style for it and calling
+// MSW native IsDialogMessage() function instead.
+#if defined(__WXMSW__) && !wxCHECK_VERSION(2, 9, 3)
+    #define USE_MSW_RETURN_KEY_WORKAROUND
+
+    #include <wx/msw/private.h>
+#endif // wxMSW <= 2.9.3
+
 namespace
 {
 class DurationModeChoice
@@ -202,6 +213,11 @@ class InputSequenceEditor
 
     virtual void EndModal(int retCode);
 
+#ifdef USE_MSW_RETURN_KEY_WORKAROUND
+  protected:
+    virtual bool MSWProcessMessage(WXMSG* pMsg);
+#endif // USE_MSW_RETURN_KEY_WORKAROUND
+
   private:
     void add_row();
     void insert_row(int row);
@@ -335,6 +351,12 @@ InputSequenceEditor::InputSequenceEditor(wxWindow* parent, 
wxString const& title
     add_row();
 
     value_field_ctrl(0).SetFocus();
+
+#ifdef USE_MSW_RETURN_KEY_WORKAROUND
+    // Turn off wx support for keyboard navigation as we use our own version in
+    // the overridden MSWProcessMessage().
+    ToggleWindowStyle(wxTAB_TRAVERSAL);
+#endif // USE_MSW_RETURN_KEY_WORKAROUND
 }
 
 void InputSequenceEditor::sequence(InputSequence const& s)
@@ -550,6 +572,16 @@ void InputSequenceEditor::insert_row(int new_row)
     sizer_->wxSizer::Insert(insert_pos++, then_label, flags);
     SizeWinForText(then_label, LARGEST_THEN_TEXT);
 
+#ifdef USE_MSW_RETURN_KEY_WORKAROUND
+    // wxMSW (mistakenly, but it's too late to change this for 2.9.2 which is
+    // the only release for which we use this workaround anyhow) uses 
WS_TABSTOP
+    // style for the spin button part of spin control and this prevents the
+    // native TAB navigation from working, so forcefully reset this style.
+    LONG msw_style = ::GetWindowLong(GetHwndOf(duration_num), GWL_STYLE);
+    msw_style &= ~WS_TABSTOP;
+    ::SetWindowLong(GetHwndOf(duration_num), GWL_STYLE, msw_style);
+#endif // USE_MSW_RETURN_KEY_WORKAROUND
+
 #undef LARGEST_FROM_TEXT
 #undef LARGEST_THEN_TEXT
 
@@ -1164,6 +1196,20 @@ void InputSequenceEditor::EndModal(int retCode)
     wxDialog::EndModal(retCode);
 }
 
+#ifdef USE_MSW_RETURN_KEY_WORKAROUND
+
+bool InputSequenceEditor::MSWProcessMessage(WXMSG* pMsg)
+{
+    if(::IsDialogMessage(GetHwnd(), pMsg))
+        {
+        return true;
+        }
+
+    return wxDialog::MSWProcessMessage(pMsg);
+}
+
+#endif // USE_MSW_RETURN_KEY_WORKAROUND
+
 class InputSequenceTextCtrl
     :public wxTextCtrl
 {