Accessibility: Fixed some bugs in Windows text navigation and readouts, improved selection and focus navigation

modules/juce_gui_basics/native/accessibility/juce_win32_Accessibility.cpp

@@ -133,15 +133,24 @@ void sendAccessibilityPropertyChangedEvent (const AccessibilityHandler& handler,
 
 void notifyAccessibilityEventInternal (const AccessibilityHandler& handler, InternalAccessibilityEvent eventType)
 {
+    if (eventType == InternalAccessibilityEvent::elementCreated
+        || eventType == InternalAccessibilityEvent::elementDestroyed)
+    {
+        if (auto* parent = handler.getParent())
+            sendAccessibilityAutomationEvent (*parent, UIA_LayoutInvalidatedEventId);
+
+        return;
+    }
+
     auto event = [eventType]() -> EVENTID
     {
         switch (eventType)
         {
-            case InternalAccessibilityEvent::elementCreated:
-            case InternalAccessibilityEvent::elementDestroyed:  return UIA_StructureChangedEventId;
             case InternalAccessibilityEvent::focusChanged:      return UIA_AutomationFocusChangedEventId;
             case InternalAccessibilityEvent::windowOpened:      return UIA_Window_WindowOpenedEventId;
             case InternalAccessibilityEvent::windowClosed:      return UIA_Window_WindowClosedEventId;
+            case InternalAccessibilityEvent::elementCreated:
+            case InternalAccessibilityEvent::elementDestroyed:  break;
         }
 
         return {};

modules/juce_gui_basics/native/accessibility/juce_win32_AccessibilityElement.cpp

@@ -87,12 +87,6 @@ static long roleToControlTypeId (AccessibilityRole roleType)
     return UIA_CustomControlTypeId;
 }
 
-static bool isEditableText (const AccessibilityHandler& handler)
-{
-    return handler.getRole() == AccessibilityRole::editableText
-          && handler.getTextInterface() != nullptr;
-}
-
 //==============================================================================
 AccessibilityNativeHandle::AccessibilityNativeHandle (AccessibilityHandler& handler)
     : ComBaseClassHelper (0),
@@ -171,10 +165,12 @@ JUCE_COMRESULT AccessibilityNativeHandle::GetPatternProvider (PATTERNID pId, IUn
                 }
                 case UIA_ValuePatternId:
                 {
-                    auto editableText = isEditableText (accessibilityHandler);
+                    if (accessibilityHandler.getValueInterface() != nullptr
-
+                        || isEditableText (accessibilityHandler)
-                    if (accessibilityHandler.getValueInterface() != nullptr || editableText)
+                        || nameIsAccessibilityValue (role))
-                        return new UIAValueProvider (this, editableText);
+                    {
+                        return new UIAValueProvider (this);
+                    }
 
                     break;
                 }
@@ -223,21 +219,15 @@ JUCE_COMRESULT AccessibilityNativeHandle::GetPatternProvider (PATTERNID pId, IUn
                 }
                 case UIA_GridPatternId:
                 {
-                    if ((role == AccessibilityRole::table || role == AccessibilityRole::tree)
+                    if (accessibilityHandler.getTableInterface() != nullptr)
-                        && accessibilityHandler.getTableInterface() != nullptr)
-                    {
                         return new UIAGridProvider (this);
-                    }
 
                     break;
                 }
                 case UIA_GridItemPatternId:
                 {
-                    if ((role == AccessibilityRole::cell || role == AccessibilityRole::treeItem)
+                    if (accessibilityHandler.getCellInterface() != nullptr)
-                        && accessibilityHandler.getCellInterface() != nullptr)
-                    {
                         return new UIAGridItemProvider (this);
-                    }
 
                     break;
                 }
@@ -250,11 +240,8 @@ JUCE_COMRESULT AccessibilityNativeHandle::GetPatternProvider (PATTERNID pId, IUn
                 }
                 case UIA_ExpandCollapsePatternId:
                 {
-                    if (role == AccessibilityRole::menuItem
+                    if (accessibilityHandler.getActions().contains (AccessibilityActionType::showMenu))
-                        && accessibilityHandler.getActions().contains (AccessibilityActionType::showMenu))
-                    {
                         return new UIAExpandCollapseProvider (this);
-                    }
 
                     break;
                 }
@@ -277,14 +264,16 @@ JUCE_COMRESULT AccessibilityNativeHandle::GetPropertyValue (PROPERTYID propertyI
 
         const auto fragmentRoot = isFragmentRoot();
 
+        const auto role = accessibilityHandler.getRole();
+        const auto state = accessibilityHandler.getCurrentState();
+
         switch (propertyId)
         {
             case UIA_AutomationIdPropertyId:
                 VariantHelpers::setString (getAutomationId (accessibilityHandler), pRetVal);
                 break;
             case UIA_ControlTypePropertyId:
-                VariantHelpers::setInt (roleToControlTypeId (accessibilityHandler.getRole()),
+                VariantHelpers::setInt (roleToControlTypeId (role), pRetVal);
-                                        pRetVal);
                 break;
             case UIA_FrameworkIdPropertyId:
                 VariantHelpers::setString ("JUCE", pRetVal);
@@ -296,25 +285,26 @@ JUCE_COMRESULT AccessibilityNativeHandle::GetPropertyValue (PROPERTYID propertyI
                 VariantHelpers::setString (accessibilityHandler.getHelp(), pRetVal);
                 break;
             case UIA_IsContentElementPropertyId:
-                VariantHelpers::setBool (! accessibilityHandler.isIgnored(), pRetVal);
+                VariantHelpers::setBool (! accessibilityHandler.isIgnored() && accessibilityHandler.isVisibleWithinParent(),
+                                         pRetVal);
                 break;
             case UIA_IsControlElementPropertyId:
                 VariantHelpers::setBool (true, pRetVal);
                 break;
             case UIA_IsDialogPropertyId:
-                VariantHelpers::setBool (accessibilityHandler.getRole() == AccessibilityRole::dialogWindow, pRetVal);
+                VariantHelpers::setBool (role == AccessibilityRole::dialogWindow, pRetVal);
                 break;
             case UIA_IsEnabledPropertyId:
                 VariantHelpers::setBool (accessibilityHandler.getComponent().isEnabled(), pRetVal);
                 break;
             case UIA_IsKeyboardFocusablePropertyId:
-                VariantHelpers::setBool (accessibilityHandler.getCurrentState().isFocusable(), pRetVal);
+                VariantHelpers::setBool (state.isFocusable(), pRetVal);
                 break;
             case UIA_HasKeyboardFocusPropertyId:
                 VariantHelpers::setBool (accessibilityHandler.hasFocus (true), pRetVal);
                 break;
             case UIA_IsOffscreenPropertyId:
-                VariantHelpers::setBool (false, pRetVal);
+                VariantHelpers::setBool (! accessibilityHandler.isVisibleWithinParent(), pRetVal);
                 break;
             case UIA_IsPasswordPropertyId:
                 if (auto* textInterface = accessibilityHandler.getTextInterface())
@@ -322,9 +312,9 @@ JUCE_COMRESULT AccessibilityNativeHandle::GetPropertyValue (PROPERTYID propertyI
 
                 break;
             case UIA_IsPeripheralPropertyId:
-                VariantHelpers::setBool (accessibilityHandler.getRole() == AccessibilityRole::tooltip
+                VariantHelpers::setBool (role == AccessibilityRole::tooltip
-                                         || accessibilityHandler.getRole() == AccessibilityRole::popupMenu
+                                         || role == AccessibilityRole::popupMenu
-                                         || accessibilityHandler.getRole() == AccessibilityRole::splashScreen,
+                                         || role == AccessibilityRole::splashScreen,
                                          pRetVal);
                 break;
             case UIA_NamePropertyId:
@@ -451,7 +441,12 @@ JUCE_COMRESULT AccessibilityNativeHandle::SetFocus()
     if (! isElementValid())
         return UIA_E_ELEMENTNOTAVAILABLE;
 
-    accessibilityHandler.grabFocus();
+    const WeakReference<Component> safeComponent (&accessibilityHandler.getComponent());
+
+    accessibilityHandler.getActions().invoke (AccessibilityActionType::focus);
+
+    if (safeComponent != nullptr)
+        accessibilityHandler.grabFocus();
 
     return S_OK;
 }
@@ -544,7 +539,12 @@ JUCE_COMRESULT AccessibilityNativeHandle::GetFocus (IRawElementProviderFragment*
 //==============================================================================
 String AccessibilityNativeHandle::getElementName() const
 {
-    if (accessibilityHandler.getRole() == AccessibilityRole::tooltip)
+    const auto role = accessibilityHandler.getRole();
+
+    if (nameIsAccessibilityValue (role))
+        return {};
+
+    if (role == AccessibilityRole::tooltip)
         return accessibilityHandler.getDescription();
 
     auto name = accessibilityHandler.getTitle();

modules/juce_gui_basics/native/accessibility/juce_win32_UIAHelpers.h

@@ -58,7 +58,7 @@ namespace VariantHelpers
     }
 }
 
-JUCE_COMRESULT addHandlersToArray (const std::vector<const AccessibilityHandler*>& handlers, SAFEARRAY** pRetVal)
+inline JUCE_COMRESULT addHandlersToArray (const std::vector<const AccessibilityHandler*>& handlers, SAFEARRAY** pRetVal)
 {
     auto numHandlers = handlers.size();
 
@@ -87,7 +87,7 @@ JUCE_COMRESULT addHandlersToArray (const std::vector<const AccessibilityHandler*
 }
 
 template <typename Value, typename Object, typename Callback>
-JUCE_COMRESULT withCheckedComArgs (Value* pRetVal, Object& handle, Callback&& callback)
+inline JUCE_COMRESULT withCheckedComArgs (Value* pRetVal, Object& handle, Callback&& callback)
 {
     if (pRetVal == nullptr)
         return E_INVALIDARG;
@@ -100,4 +100,15 @@ JUCE_COMRESULT withCheckedComArgs (Value* pRetVal, Object& handle, Callback&& ca
     return callback();
 }
 
+inline bool isEditableText (const AccessibilityHandler& handler)
+{
+    return handler.getRole() == AccessibilityRole::editableText
+        && handler.getTextInterface() != nullptr;
+}
+
+inline bool nameIsAccessibilityValue (AccessibilityRole role)
+{
+    return role == AccessibilityRole::staticText;
+}
+
 } // namespace juce

modules/juce_gui_basics/native/accessibility/juce_win32_UIASelectionProvider.h

@@ -112,7 +112,7 @@ public:
 
         AddToSelection();
 
-        if (! isRadioButton)
+        if (isElementValid() && ! isRadioButton)
         {
             const auto& handler = getHandler();
 

modules/juce_gui_basics/native/accessibility/juce_win32_UIATextProvider.h

@@ -257,32 +257,30 @@ private:
                 {
                     selectionRange.setStart (jlimit (0, numCharacters - 1, selectionRange.getStart()));
                     selectionRange.setEnd (selectionRange.getStart() + 1);
-
-                    return S_OK;
                 }
-
+                else if (unit == TextUnit_Paragraph
-                if (unit == TextUnit_Paragraph
+                      || unit == TextUnit_Page
-                    || unit == TextUnit_Page
+                      || unit == TextUnit_Document)
-                    || unit == TextUnit_Document)
                 {
-                    selectionRange = { 0, textInterface->getTotalNumCharacters() };
+                    selectionRange = { 0, numCharacters };
-                    return S_OK;
                 }
-
+                else if (unit == TextUnit_Word
-                auto start = getNextEndpointPosition (*textInterface,
+                      || unit == TextUnit_Format
-                                                      selectionRange.getStart(),
+                      || unit == TextUnit_Line)
-                                                      unit,
-                                                      NextEndpointDirection::backwards);
-
-                if (start >= 0)
                 {
-                    auto end = getNextEndpointPosition (*textInterface,
+                    const auto boundaryType = (unit == TextUnit_Line ? BoundaryType::line : BoundaryType::word);
-                                                        start,
-                                                        unit,
-                                                        NextEndpointDirection::forwards);
 
-                    if (end >= 0)
+                    auto start = findBoundary (*textInterface,
-                        selectionRange = Range<int> (start, end);
+                                               selectionRange.getStart(),
+                                               boundaryType,
+                                               NextEndpointDirection::backwards);
+
+                    auto end = findBoundary (*textInterface,
+                                             start,
+                                             boundaryType,
+                                             NextEndpointDirection::forwards);
+
+                    selectionRange = Range<int> (start, end);
                 }
 
                 return S_OK;
@@ -495,12 +493,12 @@ private:
                 if (count == 0 || numCharacters == 0)
                     return S_OK;
 
-                auto isStart = (endpoint == TextPatternRangeEndpoint_Start);
+                const auto isStart = (endpoint == TextPatternRangeEndpoint_Start);
                 auto endpointToMove = (isStart ? selectionRange.getStart() : selectionRange.getEnd());
 
                 if (unit == TextUnit_Character)
                 {
-                    auto targetPoint = jlimit (0, numCharacters, endpointToMove + count);
+                    const auto targetPoint = jlimit (0, numCharacters, endpointToMove + count);
 
                     *pRetVal = targetPoint - endpointToMove;
                     setEndpointChecked (endpoint, targetPoint);
@@ -520,26 +518,31 @@ private:
                     return S_OK;
                 }
 
-                for (int i = 0; i < std::abs (count); ++i)
+                if (unit == TextUnit_Word
+                    || unit == TextUnit_Format
+                    || unit == TextUnit_Line)
                 {
-                    auto nextEndpoint = getNextEndpointPosition (textInterface,
+                    const auto boundaryType = unit == TextUnit_Line ? BoundaryType::line : BoundaryType::word;
-                                                                 endpointToMove,
-                                                                 unit,
-                                                                 direction);
 
-                    if (nextEndpoint < 0)
+                    // handle case where endpoint is on a boundary
+                    if (findBoundary (textInterface, endpointToMove, boundaryType, direction) == endpointToMove)
+                        endpointToMove += (direction == NextEndpointDirection::forwards ? 1 : -1);
+
+                    int numMoved;
+                    for (numMoved = 0; numMoved < std::abs (count); ++numMoved)
                     {
-                        *pRetVal = (direction == NextEndpointDirection::forwards ? i : -i);
+                        auto nextEndpoint = findBoundary (textInterface, endpointToMove, boundaryType, direction);
-                        setEndpointChecked (endpoint, endpointToMove);
+
-                        return S_OK;
+                        if (nextEndpoint == endpointToMove)
+                            break;
+
+                        endpointToMove = nextEndpoint;
                     }
 
-                    endpointToMove = nextEndpoint;
+                    *pRetVal = numMoved;
+                    setEndpointChecked (endpoint, endpointToMove);
                 }
 
-                *pRetVal = count;
-                setEndpointChecked (endpoint, endpointToMove);
-
                 return S_OK;
             });
         }
@@ -584,52 +587,27 @@ private:
 
     private:
         enum class NextEndpointDirection { forwards, backwards };
+        enum class BoundaryType { word, line };
 
-        static int getNextEndpointPosition (const AccessibilityTextInterface& textInterface,
+        static int findBoundary (const AccessibilityTextInterface& textInterface,
-                                            int currentPosition,
+                                 int currentPosition,
-                                            TextUnit unit,
+                                 BoundaryType boundary,
-                                            NextEndpointDirection direction)
+                                 NextEndpointDirection direction)
         {
-            auto isTextUnitSeparator = [unit] (const juce_wchar c)
+            const auto text = [&]() -> String
             {
-                return ((unit == TextUnit_Word || unit == TextUnit_Format) && CharacterFunctions::isWhitespace (c))
+                if (direction == NextEndpointDirection::forwards)
-                       || (unit == TextUnit_Line && (c == '\r' || c == '\n'));
+                    return textInterface.getText ({ currentPosition, textInterface.getTotalNumCharacters() });
-            };
 
-            constexpr int textBufferSize = 1024;
+                auto stdString = textInterface.getText ({ 0, currentPosition }).toStdString();
-            int numChars = 0;
+                std::reverse (stdString.begin(), stdString.end());
+                return stdString;
+            }();
 
-            if (direction == NextEndpointDirection::forwards)
+            auto tokens = (boundary == BoundaryType::line ? StringArray::fromLines (text)
-            {
+                                                          : StringArray::fromTokens (text, false));
-                auto textBuffer = textInterface.getText ({ currentPosition,
-                                                           jmin (textInterface.getTotalNumCharacters(), currentPosition + textBufferSize) });
 
-                for (auto charPtr = textBuffer.getCharPointer(); ! charPtr.isEmpty();)
+            return currentPosition + (direction == NextEndpointDirection::forwards ? tokens[0].length() : -(tokens[0].length()));
-                {
-                    auto character = charPtr.getAndAdvance();
-                    ++numChars;
-
-                    if (isTextUnitSeparator (character))
-                        return currentPosition + numChars;
-                }
-            }
-            else
-            {
-                auto textBuffer = textInterface.getText ({ jmax (0, currentPosition - textBufferSize),
-                                                           currentPosition });
-
-                for (auto charPtr = textBuffer.end() - 1; charPtr != textBuffer.begin(); --charPtr)
-                {
-                    auto character = *charPtr;
-
-                    if (isTextUnitSeparator (character))
-                        return currentPosition - numChars;
-
-                    ++numChars;
-                }
-            }
-
-            return -1;
         }
 
         void setEndpointChecked (TextPatternRangeEndpoint endpoint, int newEndpoint)

modules/juce_gui_basics/native/accessibility/juce_win32_UIAValueProvider.h

@@ -31,9 +31,8 @@ class UIAValueProvider  : public UIAProviderBase,
                           public ComBaseClassHelper<IValueProvider>
 {
 public:
-    UIAValueProvider (AccessibilityNativeHandle* nativeHandle, bool editableText)
+    UIAValueProvider (AccessibilityNativeHandle* nativeHandle)
-        : UIAProviderBase (nativeHandle),
+        : UIAProviderBase (nativeHandle)
-          isEditableText (editableText)
     {
     }
 
@@ -45,6 +44,9 @@ public:
 
         const auto& handler = getHandler();
 
+        if (nameIsAccessibilityValue (handler.getRole()))
+            return UIA_E_NOTSUPPORTED;
+
         const auto sendValuePropertyChangeMessage = [&]()
         {
             VARIANT newValue;
@@ -53,7 +55,7 @@ public:
             sendAccessibilityPropertyChangedEvent (handler, UIA_ValueValuePropertyId, newValue);
         };
 
-        if (isEditableText)
+        if (isEditableText (handler))
         {
             handler.getTextInterface()->setText (String (val));
             sendValuePropertyChangeMessage();
@@ -90,9 +92,16 @@ public:
     {
         return withCheckedComArgs (pRetVal, *this, [&]
         {
-            if (! isEditableText)
+            *pRetVal = true;
-                if (auto* valueInterface = getHandler().getValueInterface())
+
-                    *pRetVal = valueInterface->isReadOnly();
+            const auto& handler = getHandler();
+
+            if (isEditableText (handler)
+                || (handler.getValueInterface() != nullptr
+                    && ! handler.getValueInterface()->isReadOnly()))
+            {
+                *pRetVal = false;
+            }
 
             return S_OK;
         });
@@ -101,7 +110,12 @@ public:
 private:
     String getCurrentValueString() const
     {
-        if (isEditableText)
+        const auto& handler = getHandler();
+
+        if (nameIsAccessibilityValue (handler.getRole()))
+            return handler.getTitle();
+
+        if (isEditableText (handler))
             if (auto* textInterface = getHandler().getTextInterface())
                 return textInterface->getText ({ 0, textInterface->getTotalNumCharacters() });
 
@@ -112,8 +126,6 @@ private:
         return {};
     }
 
-    const bool isEditableText;
-
     //==============================================================================
     JUCE_DECLARE_NON_COPYABLE_WITH_LEAK_DETECTOR (UIAValueProvider)
 };