Bug 2251 - Windows: NVDA sometimes reads the name of the track twice

Problem:
Currently calling Track::EnsureVisible() also sets the track as focus.
In Audacity 2.3.3 the timing of the code to set the focus was changed. Rather than a direct call, an event is queued, and then the focus is set. This has changed the timing of the focus event which is sent with respect to other focus and name change events. In particular in the case of toggling the selectness of the focused track, this moved the focus event to be after the name change event.
These changes only had an effect on NVDA - Jaws and Narrator were unaffected.

The introduction of this bug has highlighted an existing problem.
1. There are a small number of existing cases where a track needs to be visible, but where it is already the focus, and so setting the focus is unnecessary. For example, pressing Enter to toggle whether a track is selected.
2. Some of the Audacity code which calls EnsureVisible() is written with the assumption that EnsureVisible() doesn't set the focus, and so there are unnecessary focus events. Whilst other code which calls EnsureVisible() assumes that it also sets the focus. Confusion.

The Fix:
Remove the setting of focus from within Track::EnsureVisible(), and so remove the unnecessary focus events.
Calls to set the focus were added before calls to EnsureVisible where the code was relying on EnsureVisible to set the focus. In TrackPanel::ProcessUIHandleResult, and TrackPanel::OnMouseEvent, I wasn't sure if the focus needed to be set, so called it anyway to ensure that the behaviour did not change.

So I would like to remove the setting of focus from within Track::EnsureVisible(), and add explicit calls to set the focus where necessary.
I think this would make the code clearer, remove unnecessary calls to set the focus, and make it easier to keep NVDA happy.

src/ProjectWindow.cpp

@@ -19,6 +19,7 @@ Paul Licameli split from AudacityProject.cpp
 #include "ProjectStatus.h"
 #include "RefreshCode.h"
 #include "TrackPanelMouseEvent.h"
+#include "TrackPanelAx.h"
 #include "UndoManager.h"
 #include "ViewInfo.h"
 #include "WaveClip.h"
@@ -1527,8 +1528,10 @@ void ProjectWindow::ZoomAfterImport(Track *pTrack)
       pTrack = *tracks.Selected().begin();
    if (!pTrack)
       pTrack = *tracks.Any().begin();
-   if (pTrack)
+   if (pTrack) {
+      TrackFocus::Get(project).Set(pTrack);
       pTrack->EnsureVisible();
+   }
 }
 
 // Utility function called by other zoom methods

src/TrackPanel.cpp

@@ -628,8 +628,10 @@ void TrackPanel::ProcessUIHandleResult
    if (refreshResult & Resize)
       panel->GetListener()->TP_HandleResize();
 
-   if ((refreshResult & RefreshCode::EnsureVisible) && pClickedTrack)
+   if ((refreshResult & RefreshCode::EnsureVisible) && pClickedTrack) {
+      TrackFocus::Get(*GetProject()).Set(pClickedTrack);
       pClickedTrack->EnsureVisible();
+   }
 }
 
 void TrackPanel::HandlePageUpKey()
@@ -763,8 +765,10 @@ void TrackPanel::OnMouseEvent(wxMouseEvent & event)
       this->CallAfter( [this, event]{
          const auto foundCell = FindCell(event.m_x, event.m_y);
          const auto t = FindTrack( foundCell.pCell.get() );
-         if ( t )
+         if ( t ) {
+            TrackFocus::Get(*GetProject()).Set(t.get());
             t->EnsureVisible();
+         }
       } );
    }
 
@@ -979,8 +983,6 @@ void TrackPanel::OnEnsureVisible(TrackListEvent & e)
    auto pTrack = e.mpTrack.lock();
    auto t = pTrack.get();
 
-   TrackFocus::Get( *GetProject() ).Set( t );
-
    int trackTop = 0;
    int trackHeight =0;
 

src/TrackUtilities.cpp

@@ -49,9 +49,11 @@ void DoRemoveTracks( AudacityProject &project )
          f = t;
    }
 
-   // If we actually have something left, then make sure it's seen
-   if (f)
+   // If we actually have something left, then set focus and make sure it's seen
+   if (f) {
+      TrackFocus::Get(project).Set(f);
       f->EnsureVisible();
+   }
 
    ProjectHistory::Get( project )
       .PushState(_("Removed audio track(s)"), _("Remove Track"));

src/effects/EffectManager.cpp

@@ -48,6 +48,7 @@ effects.
 #include "../ProjectWindow.h"
 #include "../SelectUtilities.h"
 #include "../TrackPanel.h"
+#include "../TrackPanelAx.h"
 #include "../ViewInfo.h"
 #include "../WaveTrack.h"
 
@@ -227,8 +228,10 @@ void EffectManager::UnregisterEffect(const PluginID & ID)
       auto pTrack = *tracks.Selected().begin();
       if (!pTrack)
          pTrack = *tracks.Any().begin();
-      if (pTrack)
+      if (pTrack) {
+         TrackFocus::Get(project).Set(pTrack);
          pTrack->EnsureVisible();
+      }
    }
 
    return true;

src/menus/EditMenus.cpp

@@ -13,6 +13,7 @@
 #include "../SelectUtilities.h"
 #include "../TimeTrack.h"
 #include "../TrackPanel.h"
+#include "../TrackPanelAx.h"
 #include "../UndoManager.h"
 #include "../ViewInfo.h"
 #include "../WaveTrack.h"
@@ -154,8 +155,10 @@ bool DoPasteNothingSelected(AudacityProject &project)
       ProjectHistory::Get( project )
          .PushState(_("Pasted from the clipboard"), _("Paste"));
 
-      if (pFirstNewTrack)
+      if (pFirstNewTrack) {
+         TrackFocus::Get(project).Set(pFirstNewTrack);
          pFirstNewTrack->EnsureVisible();
+      }
 
       return true;
    }
@@ -194,8 +197,10 @@ void OnUndo(const CommandContext &context)
    auto t = *tracks.Selected().begin();
    if (!t)
       t = *tracks.Any().begin();
-   if (t)
+   if (t) {
+      TrackFocus::Get(project).Set(t);
       t->EnsureVisible();
+   }
 }
 
 void OnRedo(const CommandContext &context)
@@ -222,8 +227,10 @@ void OnRedo(const CommandContext &context)
    auto t = *tracks.Selected().begin();
    if (!t)
       t = *tracks.Any().begin();
-   if (t)
+   if (t) {
+      TrackFocus::Get(project).Set(t);
       t->EnsureVisible();
+   }
 }
 
 void OnCut(const CommandContext &context)
@@ -630,8 +637,10 @@ void OnPaste(const CommandContext &context)
       ProjectHistory::Get( project )
          .PushState(_("Pasted from the clipboard"), _("Paste"));
 
-      if (ff)
+      if (ff) {
+         TrackFocus::Get(project).Set(ff);
          ff->EnsureVisible();
+      }
    }
 }
 

src/menus/LabelMenus.cpp

@@ -79,6 +79,7 @@ int DoAddLabel(
    ProjectHistory::Get( project ).PushState(_("Added label"), _("Label"));
 
    if (!useDialog) {
+      TrackFocus::Get(project).Set(lt);
       lt->EnsureVisible();
    }
    trackPanel.SetFocus();
@@ -337,6 +338,7 @@ void OnPasteNewLabel(const CommandContext &context)
    // plt should point to the last label track pasted to -- ensure it's visible
    // and set focus
    if (plt) {
+      TrackFocus::Get(project).Set(plt);
       plt->EnsureVisible();
       trackPanel.SetFocus();
    }

src/menus/TrackMenus.cpp

@@ -587,6 +587,7 @@ void OnNewWaveTrack(const CommandContext &context)
    ProjectHistory::Get( project )
       .PushState(_("Created new audio track"), _("New Track"));
 
+   TrackFocus::Get(project).Set(t);
    t->EnsureVisible();
 }
 
@@ -614,6 +615,7 @@ void OnNewStereoTrack(const CommandContext &context)
    ProjectHistory::Get( project )
       .PushState(_("Created new stereo audio track"), _("New Track"));
 
+   TrackFocus::Get(project).Set(left);
    left->EnsureVisible();
 }
 
@@ -633,6 +635,7 @@ void OnNewLabelTrack(const CommandContext &context)
    ProjectHistory::Get( project )
       .PushState(_("Created new label track"), _("New Track"));
 
+   TrackFocus::Get(project).Set(t);
    t->EnsureVisible();
 }
 
@@ -657,6 +660,7 @@ void OnNewTimeTrack(const CommandContext &context)
    ProjectHistory::Get( project )
       .PushState(_("Created new time track"), _("New Track"));
 
+   TrackFocus::Get(project).Set(t);
    t->EnsureVisible();
 }