From 02db402b5491e3f321ec0412a7e8df4133bc9ab7 Mon Sep 17 00:00:00 2001 From: Paul Licameli Date: Sun, 10 Feb 2019 10:31:10 -0500 Subject: [PATCH] Bug2064: ESC key should abort drags in time ruler on Linux... ... Reimplement the ESC key handling in TrackPanel and time ruler on all operating systems so that it does not rely on the focused window, but instead uses the application-wide event filter. This includes reversion of 9491605cfc8a7d60117365884fd494996b5ebbaf --- src/AdornedRulerPanel.cpp | 18 +++++++--- src/AdornedRulerPanel.h | 14 ++++++-- src/CellularPanel.cpp | 73 ++++++++++++++++++++++++--------------- src/CellularPanel.h | 6 ++-- src/Project.cpp | 5 --- src/Project.h | 6 ---- src/TrackPanel.cpp | 8 ++--- src/TrackPanel.h | 2 -- 8 files changed, 75 insertions(+), 57 deletions(-) diff --git a/src/AdornedRulerPanel.cpp b/src/AdornedRulerPanel.cpp index e36ccb6d2..906936326 100644 --- a/src/AdornedRulerPanel.cpp +++ b/src/AdornedRulerPanel.cpp @@ -2077,6 +2077,19 @@ void AdornedRulerPanel::GetMaxSize(wxCoord *width, wxCoord *height) mRuler.GetMaxSize(width, height); } +bool AdornedRulerPanel::s_AcceptsFocus{ false }; + +auto AdornedRulerPanel::TemporarilyAllowFocus() -> TempAllowFocus { + s_AcceptsFocus = true; + return TempAllowFocus{ &s_AcceptsFocus }; +} + +void AdornedRulerPanel::SetFocusFromKbd() +{ + auto temp = TemporarilyAllowFocus(); + SetFocus(); +} + // Second-level subdivision includes quick-play region and maybe the scrub bar // and also shaves little margins above and below struct AdornedRulerPanel::Subgroup final : TrackPanelGroup { @@ -2150,11 +2163,6 @@ void AdornedRulerPanel::UpdateStatusMessage( const wxString &message ) GetProject()->TP_DisplayStatusMessage(message); } -bool AdornedRulerPanel::TakesFocus() const -{ - return false; -} - void AdornedRulerPanel::CreateOverlays() { if (!mOverlay) { diff --git a/src/AdornedRulerPanel.h b/src/AdornedRulerPanel.h index 1d2c9fe00..fe8a28d16 100644 --- a/src/AdornedRulerPanel.h +++ b/src/AdornedRulerPanel.h @@ -40,6 +40,10 @@ public: ~AdornedRulerPanel(); + bool AcceptsFocus() const override { return s_AcceptsFocus; } + bool AcceptsFocusFromKeyboard() const override { return true; } + void SetFocusFromKbd() override; + public: int GetRulerHeight() { return GetRulerHeight(mShowScrubbing); } static int GetRulerHeight(bool showScrubBar); @@ -91,6 +95,14 @@ public: void DoDrawIndicator(wxDC * dc, wxCoord xx, bool playing, int width, bool scrub, bool seek); void UpdateButtonStates(); +private: + static bool s_AcceptsFocus; + struct Resetter { void operator () (bool *p) const { if(p) *p = false; } }; + using TempAllowFocus = std::unique_ptr; + +public: + static TempAllowFocus TemporarilyAllowFocus(); + private: void DoDrawPlayRegion(wxDC * dc); @@ -186,8 +198,6 @@ private: void UpdateStatusMessage( const wxString & ) override; - bool TakesFocus() const override; - void CreateOverlays(); // Cooperating objects diff --git a/src/CellularPanel.cpp b/src/CellularPanel.cpp index e7761fc1c..477dae532 100644 --- a/src/CellularPanel.cpp +++ b/src/CellularPanel.cpp @@ -37,6 +37,42 @@ #include "HitTestResult.h" #include "RefreshCode.h" +// A singleton class that intercepts escape key presses when some cellular +// panel is dragging +struct CellularPanel::Filter : wxEventFilter +{ + Filter() + { + wxEvtHandler::AddFilter( this ); + } + + ~Filter() + { + wxEvtHandler::RemoveFilter( this ); + } + + static void Create() + { + static Filter instance; + } + + int FilterEvent( wxEvent &event ) override + { + if ( spActivePanel && + event.GetEventType() == wxEVT_KEY_DOWN && + static_cast< wxKeyEvent& >( event ).GetKeyCode() == WXK_ESCAPE ) { + spActivePanel->HandleEscapeKey( true ); + return Event_Processed; + } + else + return Event_Skip; + } + + static wxWeakRef< CellularPanel > spActivePanel; +}; + +wxWeakRef< CellularPanel > CellularPanel::Filter::spActivePanel = nullptr; + struct CellularPanel::State { UIHandlePtr mUIHandle; @@ -76,6 +112,10 @@ CellularPanel::CellularPanel( , mViewInfo( viewInfo ) , mState{ std::make_unique() } { + // Create the global event filter instance for CellularPanels only when the + // first CellularPanel is created, not sooner, so that the filter will be + // invoked before that for the application. + Filter::Create(); } CellularPanel::~CellularPanel() = default; @@ -109,16 +149,8 @@ void CellularPanel::Uncapture(bool escaping, wxMouseState *pState) ReleaseMouse(); HandleMotion( *pState ); - if ( escaping -#ifndef __WXGTK__ - // See other comment in HandleClick() - || !TakesFocus() -#endif - ) { - auto lender = GetProject()->mFocusLender.get(); - if (lender) - lender->SetFocus(); - } + if ( escaping || !AcceptsFocus() ) + Filter::spActivePanel = nullptr; } bool CellularPanel::CancelDragging( bool escaping ) @@ -523,6 +555,7 @@ void CellularPanel::OnKeyDown(wxKeyEvent & event) switch (event.GetKeyCode()) { case WXK_ESCAPE: + // This switch case is now redundant with the global filter if(HandleEscapeKey(true)) // Don't skip the event, eat it so that // AudacityApp does not also stop any playback. @@ -806,16 +839,9 @@ void CellularPanel::HandleClick( const TrackPanelMouseEvent &tpmEvent ) if (refreshResult & RefreshCode::Cancelled) state.mUIHandle.reset(), handle.reset(), ClearTargets(); else { - if( !HasFocus() -#ifdef __WXGTK__ - // Bug 2056 residual - // Don't take focus even temporarily in the time ruler, because - // the restoring of it doesn't work as expected for reasons not - // yet clear. - // The price we pay is that ESC can't abort drags in the time ruler - && TakesFocus() -#endif - ) + Filter::spActivePanel = this; + + if( !HasFocus() && AcceptsFocus() ) SetFocusIgnoringChildren(); state.mpClickedCell = pCell; @@ -857,18 +883,11 @@ void CellularPanel::DoContextMenu( TrackPanelCell *pCell ) void CellularPanel::OnSetFocus(wxFocusEvent &event) { - auto &ptr = GetProject()->mFocusLender; - if ( !ptr ) - ptr = event.GetWindow(); - SetFocusedCell(); } void CellularPanel::OnKillFocus(wxFocusEvent & WXUNUSED(event)) { - // Forget any borrowing of focus - GetProject()->mFocusLender = NULL; - if (AudacityProject::HasKeyboardCapture(this)) { AudacityProject::ReleaseKeyboard(this); diff --git a/src/CellularPanel.h b/src/CellularPanel.h index 6f8f37e9a..041bbe569 100644 --- a/src/CellularPanel.h +++ b/src/CellularPanel.h @@ -57,10 +57,6 @@ public: virtual void UpdateStatusMessage( const wxString & ) = 0; - // Whether this panel keeps focus after a click and drag, or only borrows - // it. - virtual bool TakesFocus() const = 0; - public: // Structure and functions for generalized visitation of the subdivision struct Visitor { @@ -163,6 +159,8 @@ protected: private: struct State; std::unique_ptr mState; + + struct Filter; DECLARE_EVENT_TABLE() }; diff --git a/src/Project.cpp b/src/Project.cpp index aed2ed4bb..2a45b516f 100644 --- a/src/Project.cpp +++ b/src/Project.cpp @@ -5576,11 +5576,6 @@ void AudacityProject::PlaybackScroller::OnTimer(wxCommandEvent &event) } } -bool AudacityProject::IsFocused( const wxWindow *window ) const -{ - return window == mFocusLender || window == wxWindow::FindFocus(); -} - LyricsWindow* AudacityProject::GetLyricsWindow(bool create) { if (create && !mLyricsWindow) diff --git a/src/Project.h b/src/Project.h index 80c4c2789..f328ecb90 100644 --- a/src/Project.h +++ b/src/Project.h @@ -830,12 +830,6 @@ public: std::shared_ptr GetBackgroundCell() const { return mBackgroundCell; } - wxWindowRef mFocusLender; - - // Return true if the window is really focused, or if focus was borrowed - // from it - bool IsFocused( const wxWindow *window ) const; - DECLARE_EVENT_TABLE() }; diff --git a/src/TrackPanel.cpp b/src/TrackPanel.cpp index 9a0d4edd2..cd77e27e3 100644 --- a/src/TrackPanel.cpp +++ b/src/TrackPanel.cpp @@ -644,11 +644,6 @@ void TrackPanel::UpdateStatusMessage( const wxString &st ) mListener->TP_DisplayStatusMessage(status); } -bool TrackPanel::TakesFocus() const -{ - return true; -} - void TrackPanel::UpdateSelectionDisplay() { // Full refresh since the label area may need to indicate @@ -1188,7 +1183,8 @@ void TrackPanel::DrawEverythingElse(TrackPanelDrawingContext &context, // if (GetFocusedTrack() != NULL) { // the highlight was reportedly drawn even when something else // was the focus and no highlight should be drawn. -RBD - if (GetFocusedTrack() != NULL && GetProject()->IsFocused( this )) { + if (GetFocusedTrack() != NULL && + wxWindow::FindFocus() == this ) { HighlightFocusedTrack(dc, focusRect); } diff --git a/src/TrackPanel.h b/src/TrackPanel.h index 8d8209bee..4594ad347 100644 --- a/src/TrackPanel.h +++ b/src/TrackPanel.h @@ -492,8 +492,6 @@ protected: void UpdateStatusMessage( const wxString &status ) override; - bool TakesFocus() const override; - // friending GetInfoCommand allow automation to get sizes of the // tracks, track control panel and such. friend class GetInfoCommand;