Skip to content

Commit

Permalink
issue audacity#5357: Change of format of a duration in a generator ca…
Browse files Browse the repository at this point in the history
…n cause crash

With a screen reader running, in one of the the built in generators, for example chirp, changing the format of the duration can crash Audacity.

Problem:
NumericTextCtrlAx::GetName(), uses NumericTextCtrl::GetFocusedField() to get the focused field. It then uses that as in index for a vector.
However, NumericTextCtrl::UpdateAutoFocus() does not update the value of mLastField, which is returned by NumericTextCtrl::GetFocusedField().
So after a change of format, GetFocusedField() can return a value which does not exist in the new format.
This can cause Audacity to crash. This happens from Audacity 3.3.0, and I don't know why it didn't in versions before this.

Possible fixes:
1. Change NumericTextCtrl::UpdateAutoFocus() so that it updates mLastField.
2. Given that NumericTextCtrl::GetFocusedField() is only used by the accessibility object, and that the focused field can be derived for NumericTextCtrl::GetFocusedDigit(), remove NumericTextCtrl::GetFocusedField() and NumericTextCtrl::mLastField.

The second fix was implemented, as it will reduce the chance of errors in this area in the future.
  • Loading branch information
DavidBailes authored and crsib committed Oct 10, 2023
1 parent 8b226a3 commit bbb8ed3
Show file tree
Hide file tree
Showing 2 changed files with 1 addition and 6 deletions.
4 changes: 1 addition & 3 deletions src/widgets/NumericTextCtrl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -156,7 +156,6 @@ NumericTextCtrl::NumericTextCtrl(
mBackgroundBitmap{},
mDigitFont{},
mLabelFont{},
mLastField(1),
mAutoPos(options.autoPos)
, mType(type)
{
Expand Down Expand Up @@ -913,7 +912,6 @@ void NumericTextCtrl::SetFieldFocus(int digit)
return;
}
mFocusedDigit = digit;
mLastField = mFormatter->GetDigitInfos()[mFocusedDigit].field + 1;

GetAccessible()->NotifyEvent(wxACC_EVENT_OBJECT_FOCUS,
this,
Expand Down Expand Up @@ -1138,7 +1136,7 @@ wxAccStatus NumericTextCtrlAx::GetName(int childId, wxString *name)
auto & mFieldValueStrings = mCtrl->mFieldValueStrings;

wxString ctrlString = mCtrl->GetString();
int field = mCtrl->GetFocusedField();
int field = mDigits[mCtrl->GetFocusedDigit()].field + 1;

// Return the entire string including the control label
// when the requested child ID is wxACC_SELF. (Mainly when
Expand Down
3 changes: 0 additions & 3 deletions src/widgets/NumericTextCtrl.h
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,6 @@ class AUDACITY_DLL_API NumericTextCtrl final
// this control returns to the program, so you can specify.
void SetInvalidValue(double invalidValue);

int GetFocusedField() { return mLastField; }
int GetFocusedDigit() { return mFocusedDigit; }

private:
Expand Down Expand Up @@ -166,8 +165,6 @@ class AUDACITY_DLL_API NumericTextCtrl final
int mHeight;
int mButtonWidth;

int mLastField;

int mFocusedDigit { 0 };

// If true, the focus will be set to the first non-zero digit
Expand Down

0 comments on commit bbb8ed3

Please sign in to comment.