From: Felix Ernst Date: Fri, 1 Nov 2024 23:39:19 +0000 (+0100) Subject: Adapt to Orca 47 X-Git-Url: https://cloud.milkyroute.net/gitweb/dolphin.git/commitdiff_plain/179e53591b726acc0c5272e728e398de41fb51c9?hp=daf5a04d6d34e40ba0ad03d778566be0c3ebea95 Adapt to Orca 47 The screen reader Orca has seen some fundamental changes between Orca 46 and Orca 47. While they are improvements overall, they do require changes to Dolphin to preserve the intended user experience for Orca users. The biggest change is perhaps that Orca will now not only announce changes to the currently focused item, but also of its parent, which means we do not need to pass focus around between file items and the main view within Dolphin, but can keep focus on the file items most of the time. This commit implements this. The only exception of when we cannot have focus on the items within the main view is when the current location is empty or not loaded yet. Only then is the focus moved to the view itself and the placeholderMessage is announced. This commit worsens the UX for users of Orca 46 or older, so this should only be merged once most users are on Orca 47 or later. --- diff --git a/src/kitemviews/accessibility/kitemlistviewaccessible.cpp b/src/kitemviews/accessibility/kitemlistviewaccessible.cpp index 2643eb302..f8c14bf4a 100644 --- a/src/kitemviews/accessibility/kitemlistviewaccessible.cpp +++ b/src/kitemviews/accessibility/kitemlistviewaccessible.cpp @@ -36,14 +36,11 @@ KItemListViewAccessible::KItemListViewAccessible(KItemListView *view_, KItemList Q_CHECK_PTR(parent); m_accessibleDelegates.resize(childCount()); - m_announceDescriptionChangeTimer = new QTimer{view_}; - m_announceDescriptionChangeTimer->setSingleShot(true); - m_announceDescriptionChangeTimer->setInterval(100); - KItemListGroupHeader::connect(m_announceDescriptionChangeTimer, &QTimer::timeout, view_, [this]() { - // The below will have no effect if one of the list items has focus and not the view itself. Still we announce the accessibility description change - // here in case the view itself has focus e.g. after tabbing there or after opening a new location. - QAccessibleEvent announceAccessibleDescriptionEvent(this, QAccessible::DescriptionChanged); - QAccessible::updateAccessibility(&announceAccessibleDescriptionEvent); + m_announceCurrentItemTimer = new QTimer{view_}; + m_announceCurrentItemTimer->setSingleShot(true); + m_announceCurrentItemTimer->setInterval(100); + KItemListGroupHeader::connect(m_announceCurrentItemTimer, &QTimer::timeout, view_, [this]() { + slotAnnounceCurrentItemTimerTimeout(); }); } @@ -70,10 +67,6 @@ void *KItemListViewAccessible::interface_cast(QAccessible::InterfaceType type) } } -void KItemListViewAccessible::modelReset() -{ -} - QAccessibleInterface *KItemListViewAccessible::accessibleDelegate(int index) const { if (index < 0 || index >= view()->model()->count()) { @@ -263,7 +256,13 @@ QString KItemListViewAccessible::text(QAccessible::Text t) const if (t != QAccessible::Description) { return QString(); } - const auto currentItem = child(controller->selectionManager()->currentItem()); + + QAccessibleInterface *currentItem = child(controller->selectionManager()->currentItem()); + + /** + * Always announce the path last because it might be very long. + * We do not need to announce the total count of items here because accessibility software like Orca alrady announces this automatically for lists. + */ if (!currentItem) { return i18nc("@info 1 states that the folder is empty and sometimes why, 2 is the full filesystem path", "%1 at location %2", @@ -271,59 +270,36 @@ QString KItemListViewAccessible::text(QAccessible::Text t) const modelRootUrl.toDisplayString()); } - const QString selectionStateString{isSelected(currentItem) ? QString() - // i18n: There is a comma at the end because this is one property in an enumeration of - // properties that a file or folder has. Accessible text for accessibility software like screen - // readers. - : i18n("not selected,")}; - - QString expandableStateString; - if (currentItem->state().expandable) { - if (currentItem->state().collapsed) { - // i18n: There is a comma at the end because this is one property in an enumeration of properties that a folder in a tree view has. - // Accessible text for accessibility software like screen readers. - expandableStateString = i18n("collapsed,"); - } else { - // i18n: There is a comma at the end because this is one property in an enumeration of properties that a folder in a tree view has. - // Accessible text for accessibility software like screen readers. - expandableStateString = i18n("expanded,"); - } - } - - const QString selectedItemCountString{selectedItemCount() > 1 - // i18n: There is a "—" at the beginning because this is a followup sentence to a text that did not properly end - // with a period. Accessible text for accessibility software like screen readers. - ? i18np("— %1 selected item", "— %1 selected items", selectedItemCount()) - : QString()}; + const int numberOfSelectedItems = selectedItemCount(); // Determine if we should announce the item layout. For end users of the accessibility tree there is an expectation that a list can be scrolled through by // pressing the "Down" key repeatedly. This is not the case in the icon view mode, where pressing "Right" or "Left" moves through the whole list of items. // Therefore we need to announce this layout when in icon view mode. - QString layoutAnnouncementString; if (auto standardView = qobject_cast(view())) { if (standardView->itemLayout() == KStandardItemListView::ItemLayout::IconsLayout) { - layoutAnnouncementString = i18nc("@info refering to a file or folder", "in a grid layout"); + if (numberOfSelectedItems < 1 || (numberOfSelectedItems == 1 && isSelected(currentItem))) { + // We do not announce the number of selected items if the only selected item is the current item + // because the selection state of the current item is already announced elsewhere. + return i18nc("@info accessibility, 1 is path", "in a grid layout in location %1", modelRootUrl.toDisplayString()); + } + return i18ncp("@info accessibility, 2 is path", + "%1 selected item in a grid layout in location %2", + "%1 selected items in a grid layout in location %2", + numberOfSelectedItems, + modelRootUrl.toDisplayString()); } } - /** - * Announce it in this order so the most important information is at the beginning and the potentially very long path at the end: - * "$currentlyFocussedItemName, $currentlyFocussedItemDescription, $currentFolderPath". - * We do not need to announce the total count of items here because accessibility software like Orca alrady announces this automatically for lists. - * Normally for list items the selection and expandadable state are also automatically announced by Orca, however we are building the accessible - * description of the view here, so we need to manually add all infomation about the current item we also want to announce. - */ - return i18nc( - "@info 1 is currentlyFocussedItemName, 2 is empty or \"not selected, \", 3 is currentlyFocussedItemDescription, 3 is currentFolderName, 4 is " - "currentFolderPath", - "%1, %2 %3 %4 %5 %6 in location %7", - currentItem->text(QAccessible::Name), - selectionStateString, - expandableStateString, - currentItem->text(QAccessible::Description), - selectedItemCountString, - layoutAnnouncementString, - modelRootUrl.toDisplayString()); + if (numberOfSelectedItems < 1 || (numberOfSelectedItems == 1 && isSelected(currentItem))) { + // We do not announce the number of selected items if the only selected item is the current item + // because the selection state of the current item is already announced elsewhere. + return i18nc("@info accessibility, 1 is path", "in location %1", modelRootUrl.toDisplayString()); + } + return i18ncp("@info accessibility, 2 is path", + "%1 selected item in location %2", + "%1 selected items in location %2", + numberOfSelectedItems, + modelRootUrl.toDisplayString()); } QRect KItemListViewAccessible::rect() const @@ -443,37 +419,74 @@ KItemListView *KItemListViewAccessible::view() const return static_cast(object()); } -void KItemListViewAccessible::announceOverallViewState(const QString &placeholderMessage) +void KItemListViewAccessible::setAccessibleFocusAndAnnounceAll() +{ + const int currentItemIndex = view()->m_controller->selectionManager()->currentItem(); + if (currentItemIndex < 0) { + // The current item is invalid (perhaps because the folder is empty), so we set the focus to the view itself instead. + QAccessibleEvent accessibleFocusInEvent(this, QAccessible::Focus); + QAccessible::updateAccessibility(&accessibleFocusInEvent); + return; + } + + QAccessibleEvent accessibleFocusInEvent(this, QAccessible::Focus); + accessibleFocusInEvent.setChild(currentItemIndex); + QAccessible::updateAccessibility(&accessibleFocusInEvent); + m_shouldAnnounceLocation = true; + announceCurrentItem(); +} + +void KItemListViewAccessible::announceNewlyLoadedLocation(const QString &placeholderMessage) { m_placeholderMessage = placeholderMessage; + m_shouldAnnounceLocation = true; - // Make sure we announce this placeholderMessage. However, do not announce it when the focus is on an unrelated object currently. - // We for example do not want to announce "Loading cancelled" when the focus is currently on an error message explaining why the loading was cancelled. - if (view()->hasFocus() || !QApplication::focusWidget() || static_cast(m_parent->object())->isAncestorOf(QApplication::focusWidget())) { - view()->setFocus(); - // If we move focus to an item and right after that the description of the item is changed, the item will be announced twice. - // We want to avoid that so we wait until after the description change was announced to move focus. - KItemListGroupHeader::connect( - m_announceDescriptionChangeTimer, - &QTimer::timeout, - view(), - [this]() { - if (view()->hasFocus() || !QApplication::focusWidget() - || static_cast(m_parent->object())->isAncestorOf(QApplication::focusWidget())) { - QAccessibleEvent accessibleFocusEvent(this, QAccessible::Focus); - QAccessible::updateAccessibility(&accessibleFocusEvent); // This accessibility update is perhaps even too important: It is generally - // the last triggered update after changing the currently viewed folder. This call makes sure that we announce the new directory in - // full. Furthermore it also serves its original purpose of making sure we announce the placeholderMessage in empty folders. - } - }, - Qt::SingleShotConnection); - if (!m_announceDescriptionChangeTimer->isActive()) { - m_announceDescriptionChangeTimer->start(); - } - } + // Changes might still be happening in the view. We (re)start the timer to make it less likely that it announces a state that is still in flux. + m_announceCurrentItemTimer->start(); +} + +void KItemListViewAccessible::announceCurrentItem() +{ + m_announceCurrentItemTimer->start(); } -void KItemListViewAccessible::announceDescriptionChange() +void KItemListViewAccessible::slotAnnounceCurrentItemTimerTimeout() { - m_announceDescriptionChangeTimer->start(); + if (!view()->hasFocus() && QApplication::focusWidget() && QApplication::focusWidget()->isVisible() + && !static_cast(m_parent->object())->isAncestorOf(QApplication::focusWidget())) { + // Something else than this view has focus, so we do not announce anything. + m_lastAnnouncedIndex = -1; // Reset this to -1 so we properly move focus to the current item the next time this method is called. + return; + } + + /// Announce the current item (or the view if there is no current item). + const int currentIndex = view()->m_controller->selectionManager()->currentItem(); + if (currentIndex < 0) { + // The current index is invalid! There might be no items in the list. Instead the list itself is announced. + m_shouldAnnounceLocation = true; + QAccessibleEvent announceEmptyViewPlaceholderMessageEvent(this, QAccessible::Focus); + QAccessible::updateAccessibility(&announceEmptyViewPlaceholderMessageEvent); + } else if (currentIndex != m_lastAnnouncedIndex) { + QAccessibleEvent announceNewlyFocusedItemEvent(this, QAccessible::Focus); + announceNewlyFocusedItemEvent.setChild(currentIndex); + QAccessible::updateAccessibility(&announceNewlyFocusedItemEvent); + } else { + QAccessibleEvent announceCurrentItemNameChangeEvent(this, QAccessible::NameChanged); + announceCurrentItemNameChangeEvent.setChild(currentIndex); + QAccessible::updateAccessibility(&announceCurrentItemNameChangeEvent); + QAccessibleEvent announceCurrentItemDescriptionChangeEvent(this, QAccessible::DescriptionChanged); + announceCurrentItemDescriptionChangeEvent.setChild(currentIndex); + QAccessible::updateAccessibility(&announceCurrentItemDescriptionChangeEvent); + } + m_lastAnnouncedIndex = currentIndex; + + /// Announce the location if we are not just moving within the same location. + if (m_shouldAnnounceLocation) { + m_shouldAnnounceLocation = false; + + QAccessibleEvent announceAccessibleDescriptionEvent1(this, QAccessible::NameChanged); + QAccessible::updateAccessibility(&announceAccessibleDescriptionEvent1); + QAccessibleEvent announceAccessibleDescriptionEvent(this, QAccessible::DescriptionChanged); + QAccessible::updateAccessibility(&announceAccessibleDescriptionEvent); + } } diff --git a/src/kitemviews/accessibility/kitemlistviewaccessible.h b/src/kitemviews/accessibility/kitemlistviewaccessible.h index 4c44b18ad..db2832435 100644 --- a/src/kitemviews/accessibility/kitemlistviewaccessible.h +++ b/src/kitemviews/accessibility/kitemlistviewaccessible.h @@ -96,21 +96,35 @@ public: KItemListView *view() const; /** - * Moves the focus to the list view itself so an overview over the state can be given. - * @param placeholderMessage the message that should be announced when no items are visible (yet). This message is mostly identical to + * Called by KItemListContainer when it passes on focus to the view. Accessible focus is then meant to go towards this accessible interface and a detailed + * announcement of the current view state (current item and overall location state) should be triggered. + */ + void setAccessibleFocusAndAnnounceAll(); + + /** + * Called multiple times while a new location is loading. A timer is restarted, and if this method has not been called for a split second, the newly loaded + * location is finally announced. + * Either the @p placeholderMessage is announced when there are no items in the view (yet), or the current item is announced together with the view state. + * + * @param placeholderMessage The message that should be announced when no items are visible (yet). This message is mostly identical to * DolphinView::m_placeholderLabel in both content and purpose. @see DolphinView::updatePlaceHolderLabel(). + * + * If there are items in the view and the placeholderMessage is therefore not visible, the current item and location is announced instead. */ - void announceOverallViewState(const QString &placeholderMessage); + void announceNewlyLoadedLocation(const QString &placeholderMessage); /** - * Announces that the description of the view has changed. The changed description will only be announced if the view has focus (from an accessibility - * point of view). This method ensures that multiple calls to this method within a small time frame will only lead to a singular announcement instead of - * multiple or already outdated ones, so calling this method instead of manually sending accessibility events for this view is preferred. + * Starts a timer that will trigger an announcement of the current item. The timer makes sure that quick changes to the current item will only lead to a + * singular announcement. This way when a new folder is loaded we only trigger a single announcement even if the items quickly change. + * + * When m_shouldAnnounceLocation is true, the current location will be announced following the announcement of the current item. + * + * If the current item is invalid, only the current location is announced, which has the responsibility of then telling why there is no valid item in the + * view. */ - void announceDescriptionChange(); + void announceCurrentItem(); -protected: - virtual void modelReset(); +private: /** * @returns a KItemListDelegateAccessible representing the file or folder at the @index. Returns nullptr for invalid indices. * If a KItemListDelegateAccessible for an index does not yet exist, it will be created. @@ -120,11 +134,37 @@ protected: KItemListSelectionManager *selectionManager() const; +private Q_SLOTS: + /** + * Is run in response to announceCurrentItem(). If the current item exists, it is announced. Otherwise the view is announced. + * Also announces some general information about the current location if it has changed recently. + */ + void slotAnnounceCurrentItemTimerTimeout(); + private: /** @see setPlaceholderMessage(). */ QString m_placeholderMessage; - QTimer *m_announceDescriptionChangeTimer; + /** + * Is started by announceCurrentItem(). + * If we announce the current item as soon as it changes, we would announce multiple items while loading a folder. + * This timer makes sure we only announce the singular currently focused item when things have settled down. + */ + QTimer *m_announceCurrentItemTimer; + + /** + * If we want announceCurrentItem() to always announce the current item, we must be aware if this is equal to the previous current item, because + * - if the accessibility focus moves to a new item, it is automatically announced, but + * - if the focus is still on the item at the same index, the focus does not technically move to a new item even if the file at that index changed, so we + * need to instead send change events for the accessible name and accessible description. + */ + int m_lastAnnouncedIndex = -1; + + /** + * Is set to true in response to announceDescriptionChange(). When true, the next time slotAnnounceCurrentItemTimerTimeout() is called the description is + * also announced. Then this bool is set to false. + */ + bool m_shouldAnnounceLocation = true; class AccessibleIdWrapper { diff --git a/src/kitemviews/kitemlistcontainer.cpp b/src/kitemviews/kitemlistcontainer.cpp index ff12aee7c..128140e2e 100644 --- a/src/kitemviews/kitemlistcontainer.cpp +++ b/src/kitemviews/kitemlistcontainer.cpp @@ -13,7 +13,7 @@ #include "private/kitemlistsmoothscroller.h" #ifndef QT_NO_ACCESSIBILITY -#include +#include "accessibility/kitemlistviewaccessible.h" #endif #include #include @@ -202,11 +202,11 @@ void KItemListContainer::focusInEvent(QFocusEvent *event) // We need to set the focus to the view or accessibility software will only announce the container (which has no information available itself). // For some reason actively setting the focus to the view needs to be delayed or the focus will immediately go back to this container. QTimer::singleShot(0, this, [this, view]() { - view->setFocus(); + if (!isAncestorOf(QApplication::focusWidget())) { + view->setFocus(); + } #ifndef QT_NO_ACCESSIBILITY - QAccessibleEvent accessibleFocusInEvent(this, QAccessible::Focus); - accessibleFocusInEvent.setChild(0); - QAccessible::updateAccessibility(&accessibleFocusInEvent); + static_cast(QAccessible::queryAccessibleInterface(view))->setAccessibleFocusAndAnnounceAll(); #endif }); } diff --git a/src/kitemviews/kitemlistcontroller.cpp b/src/kitemviews/kitemlistcontroller.cpp index 1db665f47..821e1b75f 100644 --- a/src/kitemviews/kitemlistcontroller.cpp +++ b/src/kitemviews/kitemlistcontroller.cpp @@ -467,12 +467,6 @@ bool KItemListController::keyPressEvent(QKeyEvent *event) case Qt::Key_Space: if (m_selectionBehavior == MultiSelection) { -#ifndef QT_NO_ACCESSIBILITY - // Move accessible focus to the item that is acted upon, so only the state change of this item is announced and not the whole view. - QAccessibleEvent accessibilityEvent(view(), QAccessible::Focus); - accessibilityEvent.setChild(index); - QAccessible::updateAccessibility(&accessibilityEvent); -#endif if (controlPressed) { // Toggle the selection state of the current item. m_selectionManager->endAnchoredSelection(); diff --git a/src/kitemviews/kitemlistview.cpp b/src/kitemviews/kitemlistview.cpp index 38ec6841a..d3caa5560 100644 --- a/src/kitemviews/kitemlistview.cpp +++ b/src/kitemviews/kitemlistview.cpp @@ -1244,11 +1244,6 @@ void KItemListView::slotItemsInserted(const KItemRangeList &itemRanges) if (useAlternateBackgrounds()) { updateAlternateBackgrounds(); } -#ifndef QT_NO_ACCESSIBILITY - if (QAccessible::isActive()) { // Announce that the count of items has changed. - static_cast(QAccessible::queryAccessibleInterface(this))->announceDescriptionChange(); - } -#endif } void KItemListView::slotItemsRemoved(const KItemRangeList &itemRanges) @@ -1367,11 +1362,6 @@ void KItemListView::slotItemsRemoved(const KItemRangeList &itemRanges) if (useAlternateBackgrounds()) { updateAlternateBackgrounds(); } -#ifndef QT_NO_ACCESSIBILITY - if (QAccessible::isActive()) { // Announce that the count of items has changed. - static_cast(QAccessible::queryAccessibleInterface(this))->announceDescriptionChange(); - } -#endif } void KItemListView::slotItemsMoved(const KItemRange &itemRange, const QList &movedToIndexes) @@ -1429,15 +1419,7 @@ void KItemListView::slotItemsChanged(const KItemRangeList &itemRanges, const QSe updateVisibleGroupHeaders(); doLayout(NoAnimation); } - -#ifndef QT_NO_ACCESSIBILITY - QAccessibleTableModelChangeEvent ev(this, QAccessibleTableModelChangeEvent::DataChanged); - ev.setFirstRow(itemRange.index); - ev.setLastRow(itemRange.index + itemRange.count); - QAccessible::updateAccessibility(&ev); -#endif } - doLayout(NoAnimation); } @@ -1510,19 +1492,11 @@ void KItemListView::slotCurrentChanged(int current, int previous) KItemListWidget *currentWidget = m_visibleItems.value(current, nullptr); if (currentWidget) { currentWidget->setCurrent(true); - if (hasFocus() || (previousWidget && previousWidget->hasFocus())) { - currentWidget->setFocus(); // Mostly for accessibility, because keyboard events are handled correctly either way. - } } } #ifndef QT_NO_ACCESSIBILITY - if (QAccessible::isActive()) { - if (current >= 0) { - QAccessibleEvent accessibleFocusCurrentItemEvent(this, QAccessible::Focus); - accessibleFocusCurrentItemEvent.setChild(current); - QAccessible::updateAccessibility(&accessibleFocusCurrentItemEvent); - } - static_cast(QAccessible::queryAccessibleInterface(this))->announceDescriptionChange(); + if (current != previous && QAccessible::isActive()) { + static_cast(QAccessible::queryAccessibleInterface(this))->announceCurrentItem(); } #endif } @@ -1544,14 +1518,11 @@ void KItemListView::slotSelectionChanged(const KItemSet ¤t, const KItemSet // Let the screen reader announce "selected" or "not selected" for the active item. const bool wasSelected(previous.contains(index)); if (isSelected != wasSelected) { - QAccessibleEvent accessibleSelectionChangedEvent(this, QAccessible::Selection); + QAccessibleEvent accessibleSelectionChangedEvent(this, QAccessible::SelectionAdd); accessibleSelectionChangedEvent.setChild(index); QAccessible::updateAccessibility(&accessibleSelectionChangedEvent); } } - // Usually the below does not have an effect because the view will not have focus at this moment but one of its list items. Still we announce the - // change of the accessibility description just in case the user manually moved focus up by one. - static_cast(QAccessible::queryAccessibleInterface(this))->announceDescriptionChange(); #else } Q_UNUSED(previous) diff --git a/src/views/dolphinview.cpp b/src/views/dolphinview.cpp index 2f2ff586d..55ab8a27d 100644 --- a/src/views/dolphinview.cpp +++ b/src/views/dolphinview.cpp @@ -2342,8 +2342,7 @@ void DolphinView::showLoadingPlaceholder() m_placeholderLabel->setVisible(true); #ifndef QT_NO_ACCESSIBILITY if (QAccessible::isActive()) { - auto accessibleViewInterface = static_cast(QAccessible::queryAccessibleInterface(m_view)); - accessibleViewInterface->announceOverallViewState(m_placeholderLabel->text()); + static_cast(QAccessible::queryAccessibleInterface(m_view))->announceNewlyLoadedLocation(m_placeholderLabel->text()); } #endif } @@ -2352,6 +2351,11 @@ void DolphinView::updatePlaceholderLabel() { m_showLoadingPlaceholderTimer->stop(); if (itemsCount() > 0) { +#ifndef QT_NO_ACCESSIBILITY + if (QAccessible::isActive()) { + static_cast(QAccessible::queryAccessibleInterface(m_view))->announceNewlyLoadedLocation(QString()); + } +#endif m_placeholderLabel->setVisible(false); return; } @@ -2397,8 +2401,7 @@ void DolphinView::updatePlaceholderLabel() m_placeholderLabel->setVisible(true); #ifndef QT_NO_ACCESSIBILITY if (QAccessible::isActive()) { - auto accessibleViewInterface = static_cast(QAccessible::queryAccessibleInterface(m_view)); - accessibleViewInterface->announceOverallViewState(m_placeholderLabel->text()); + static_cast(QAccessible::queryAccessibleInterface(m_view))->announceNewlyLoadedLocation(m_placeholderLabel->text()); } #endif }