From 9c9d930760fe26f7600dcfc657b1a4f68794ec56 Mon Sep 17 00:00:00 2001 From: reuk Date: Mon, 23 May 2022 19:59:57 +0100 Subject: [PATCH] Android: Fix a couple of accessibility-related crashes These crashes could be seen in the DemoRunner when selecting items in nested PopupMenu windows. --- .../accessibility/juce_AccessibilityHandler.cpp | 1 - .../juce_gui_basics/components/juce_Component.cpp | 12 ++++++++++++ modules/juce_gui_basics/menus/juce_PopupMenu.cpp | 5 +++-- .../accessibility/juce_android_Accessibility.cpp | 9 ++++++++- .../widgets/juce_ToolbarItemComponent.cpp | 2 +- 5 files changed, 24 insertions(+), 5 deletions(-) diff --git a/modules/juce_gui_basics/accessibility/juce_AccessibilityHandler.cpp b/modules/juce_gui_basics/accessibility/juce_AccessibilityHandler.cpp index 8dbf22da71..df14dffd2f 100644 --- a/modules/juce_gui_basics/accessibility/juce_AccessibilityHandler.cpp +++ b/modules/juce_gui_basics/accessibility/juce_AccessibilityHandler.cpp @@ -63,7 +63,6 @@ AccessibilityHandler::AccessibilityHandler (Component& comp, interfaces (std::move (interfacesIn)), nativeImpl (createNativeImpl (*this)) { - notifyAccessibilityEventInternal (*this, InternalAccessibilityEvent::elementCreated); } AccessibilityHandler::~AccessibilityHandler() diff --git a/modules/juce_gui_basics/components/juce_Component.cpp b/modules/juce_gui_basics/components/juce_Component.cpp index 77c1a37e97..fcc54c5c9d 100644 --- a/modules/juce_gui_basics/components/juce_Component.cpp +++ b/modules/juce_gui_basics/components/juce_Component.cpp @@ -3308,6 +3308,18 @@ AccessibilityHandler* Component::getAccessibilityHandler() || accessibilityHandler->getTypeIndex() != std::type_index (typeid (*this))) { accessibilityHandler = createAccessibilityHandler(); + + // On Android, notifying that an element was created can cause the system to request + // the accessibility node info for the new element. If we're not careful, this will lead + // to recursive calls, as each time an element is created, new node info will be requested, + // causing an element to be created, causing a new info request... + // By assigning the accessibility handler before notifying the system that an element was + // created, the if() predicate above should evaluate to false on recursive calls, + // terminating the recursion. + if (accessibilityHandler != nullptr) + notifyAccessibilityEventInternal (*accessibilityHandler, InternalAccessibilityEvent::elementCreated); + else + jassertfalse; // createAccessibilityHandler must return non-null } return accessibilityHandler.get(); diff --git a/modules/juce_gui_basics/menus/juce_PopupMenu.cpp b/modules/juce_gui_basics/menus/juce_PopupMenu.cpp index d004782804..d0633a3e6b 100644 --- a/modules/juce_gui_basics/menus/juce_PopupMenu.cpp +++ b/modules/juce_gui_basics/menus/juce_PopupMenu.cpp @@ -87,7 +87,7 @@ struct HeaderItemComponent : public PopupMenu::CustomComponent std::unique_ptr createAccessibilityHandler() override { - return nullptr; + return createIgnoredAccessibilityHandler (*this); } const Options& options; @@ -275,7 +275,8 @@ private: std::unique_ptr createAccessibilityHandler() override { - return item.isSeparator ? nullptr : std::make_unique (*this); + return item.isSeparator ? createIgnoredAccessibilityHandler (*this) + : std::make_unique (*this); } //============================================================================== diff --git a/modules/juce_gui_basics/native/accessibility/juce_android_Accessibility.cpp b/modules/juce_gui_basics/native/accessibility/juce_android_Accessibility.cpp index 53ae2a67f3..637cfbd8a8 100644 --- a/modules/juce_gui_basics/native/accessibility/juce_android_Accessibility.cpp +++ b/modules/juce_gui_basics/native/accessibility/juce_android_Accessibility.cpp @@ -481,10 +481,15 @@ public: case ACTION_CLICK: { + // Invoking the action may delete this handler + const WeakReference savedHandle { this }; + if ((accessibilityHandler.getCurrentState().isCheckable() && accessibilityHandler.getActions().invoke (AccessibilityActionType::toggle)) || accessibilityHandler.getActions().invoke (AccessibilityActionType::press)) { - sendAccessibilityEventImpl (accessibilityHandler, TYPE_VIEW_CLICKED, 0); + if (savedHandle != nullptr) + sendAccessibilityEventImpl (accessibilityHandler, TYPE_VIEW_CLICKED, 0); + return true; } @@ -742,6 +747,8 @@ private: //============================================================================== JUCE_DECLARE_NON_COPYABLE_WITH_LEAK_DETECTOR (AccessibilityNativeHandle) + + JUCE_DECLARE_WEAK_REFERENCEABLE (AccessibilityNativeHandle) }; std::unordered_map AccessibilityNativeHandle::virtualViewIdMap; diff --git a/modules/juce_gui_basics/widgets/juce_ToolbarItemComponent.cpp b/modules/juce_gui_basics/widgets/juce_ToolbarItemComponent.cpp index e42f0e6195..e871094abb 100644 --- a/modules/juce_gui_basics/widgets/juce_ToolbarItemComponent.cpp +++ b/modules/juce_gui_basics/widgets/juce_ToolbarItemComponent.cpp @@ -247,7 +247,7 @@ std::unique_ptr ToolbarItemComponent::createAccessibilityH && itemId != ToolbarItemFactory::flexibleSpacerId); if (! shouldItemBeAccessible) - return nullptr; + return createIgnoredAccessibilityHandler (*this); return std::make_unique (*this, AccessibilityRole::button); }