... note that "cat" compared with it is concatenated with _("Menu") and
stored in KeyNode::category, which is also compared elsewhere with
CommandTranslated. So this change in one comparison is the right one.
Caused by this commit: 0021a98091 : Remove another unnecessary stripping of accelerators from labels
Unfortunately, the stripping was necessary.
Fix:
Reinstate stripping.
Derives from tag: Audacity-2.3.3 + 94 commits
Closing sub trees can leave the wrong item selected.
If a node near the bottom of the tree is closed, the node may move
down, and no longer be under the mouse pointer. If further processing
is allowed, then the line at the mouse position is incorrectly selected.
Fix: Don't allow further processing and call SetFocus() ourselves.
My previous fix for this bug, commit 4b437b8, did not work on Mac.
This bug was introduced by my commit b62ed73. This commit was to ensure that when the keyview was the focus, there was always one item selected. This ensures that the focus is indicated visually, and that the Narrator screen reader reads the keyview.
The failed fix, selected an item if necessary when the keyview became the focus.
The current fix reverts b62ed73 and 4b437b8, and ensures that an item is selected each time the items are updated.
Steps to reproduce:
1. Open preferences
2. Select the keyboard category
3. Scroll down the list by any amount
4. Select an item using the mouse. The list scrolls to the top and the wrong item is selected.
The problem occurs because if the list of shortcuts is currently not the focus, then after a left mouse click, KeyView::OnSetFocus() is called, and setting the selection in that function interferes with the mouse selection.
Fix: In KeyView::OnSetFocus(), if there has been a left down event, don't select anything.
Problem. If an item in the list is selected, then if a different view by radio button is selected, Narrator reads the selected item rather than the radio button.
KeyViewAx::SetCurrentLine(int line) can be called when the KeyView is not the focus, and send a focus event.
Fix: In KeyViewAx::SetCurrentLine(int line), only send focus event if the KeyView is the focus.
... Unnecessary because transitively included.
But each .cpp file still includes its own .h file near the top to ensure
that it compiles indenendently, even if it is reincluded transitively later.
Problem: A user can tab to the list, and none of the items is the focus. The list itself is still the focus. As a result, there is no visual indication of the focus, and an additional keystroke is needed to make any item the focus.
Fix: When the list gains focus, if no items are selected, select the first item, if there is one.
... except Audacity.h
This forces us to make each header contain all forward declarations or nested
headers that it requires, rather than depend on context.
... Make all line drawing go through our AColor which wraps the wxDC
line-drawing functions.
Despite what wxWidgets documentation says, the functions still don't
consistently include the first point and omit the last point of a line. I
observed inclusion of both on Mac, while on Windows, omission sometimes of the
first point instead of the last.
Discrepancies could be observed in 2.3.0 between Windows and Mac, using the
Window magnifier or command+alt+ +, zooming in closely on the ends of the
shadow lines below and right of tracks, or at the bottom-right corners of the
bevels drawn around vertical rulers.
So where there is an observable one-pixel difference of drawing between
platforms, there is the question, which was the intent when the drawing code
was written? Should the coordinates be corrected by one or not?
I reviewed each case and used my judgment.
Most of the calls are in drawing rulers. Major tick lines were drawn five
pixels long, and minor, three, on Mac. I keep this behavior, but you might
argue for making them four and two.
On the other hand the drawing of ruler grid lines, which you can see in
the equalization and frequency analysis windows, did need a one-pixel correction
to avoid straying out of bounds.
... Replacing:
Insert => insert
RemoveAt => erase
Remove => erase
IsSameAs => operator == or operator !=
(but only when second argument was true or default)
... which will make it easier to change the types of those containers to
std::vectors of other string-like classes
for wxString,
IsEmpty => empty
Clear => clear
Alloc => reserve
for wxArrayString,
Count => size
GetCount => size
IsEmpty => empty
Add => push_back
Clear => clear
Empty => clear
Sort => std::sort (only with default comparator)
SetCount => resize
Last => back
Item => operator []
Alloc => reserve
The main change in wx accessibility is this:
7dab555f71 (diff-04f5191d86f95b1c4d5d9c979da65878)
However wxWindowAccessible has not been updated to take into account of that change. In particular wxWindowAccessible::GetParent() was always wrong, but it was consistent with the rest of the framework. Now it's wrong and inconsistent. This function should return an object with role window, and which has the same name.
The fix is to introduce class WindowAccessible, which is effectively our own version of wxWindowAccessible. This class does not override GetParent(), and so just relies on a standard accessible object to to the right thing in wxIAccessible::get_accParent() (which is does). This class also allows us to have our own version of GetName(), which allows us to set the accessibility names of buttons.
These changes will break the accessibility of Audacity if it is built with wxWidgets 3.0.X. If this is a problem, then there could be some #if stuff in WindowAccessible.h to turn the WindowAccessible class into one which simply inherits from wxWindowAccessible, and doesn't override anything.
... Such are not for display to the user. They are appended to menu item
names to identify accelerators, and wxWidgets transforms them appropriately
for the operating system.
... and similar wx "variadics," which all treat wxString smartly enough that
you don't need this.
Don't need c_str either to convert wxString to const wxChar * because
wxString has a conversion operator that does the same.
- Dead code from experiments in SelectionBar removed.
- Many warnings about unused parameters fixed with WXUNUSED()
- Many warnings about signed / unsigned comparisons cleaned up.
- Several 'local variable declared but not used' warnings fixed.
Fixed by putting key bindings on the left in all three views.
I also elevated EXPERIMENTAL_KEY_VIEW to no longer experimental, by excising the old code which we don't need any more.
We now don't sort the list until a view is selected. Also we create it with nodes open, rather than creating with nodes closed and then separately opening them. Instead of 3 sorts at initialisation only one happens now. AX info is only updated after a sort, so this may be 3x faster for screen reader users too.
The slow opening was caused by sorting lists of commands. The comparison function was slow because it created new strings, entailing malloc/free and used translation in the function. Comparison function was being called about 4,000 times.
Paul Licameli
freddii
Yuri Chornoivan
David Bailes
James Crook
Paul Licameli
Leland Lucius