Improve accessibility, don't let focusing unread bar.

Author: john-preston

Telegram/SourceFiles/history/history_inner_widget.cpp

@@ -4083,25 +4083,22 @@ void HistoryInner::keyPressEvent(QKeyEvent *e) {
 			const auto elements = accessibleElements();
 			const auto barIndex
 				= accessibilityUnreadBarIndex();
-			const auto elementIndex = (barIndex >= 0
-				&& _accessibilityFocusedIndex > barIndex)
-				? (_accessibilityFocusedIndex - 1)
-				: _accessibilityFocusedIndex;
-			if (elementIndex < 0
-				|| elementIndex >= int(elements.size())
-				|| elements[elementIndex]->data().get()
-					!= _accessibilityFocusedItem) {
+			if (accessibilityItemAtIndex(
+					_accessibilityFocusedIndex,
+					elements,
+					barIndex) != _accessibilityFocusedItem) {
 				// The focused item is still the same message, but
 				// its index in accessibleElements() shifted (the list
-				// was mutated since the last navigation). Repair the
-				// cached index in-place without emitting a focus
-				// change — the framework still thinks the focused
-				// child is _accessibilityFocusedItem and we are only
-				// catching up our bookkeeping. If the item is not in
-				// the loaded slice anymore the index is invalidated
-				// instead: selection and media actions dispatch by
-				// index, and a stale one would silently operate on
-				// whatever unrelated row occupies it now.
+				// was mutated since the last navigation, possibly by
+				// an unread bar appearing right at the cached index).
+				// Repair the cached index in-place without emitting a
+				// focus change — the framework still thinks the
+				// focused child is _accessibilityFocusedItem and we
+				// are only catching up our bookkeeping. If the item
+				// is not in the loaded slice anymore the index is
+				// invalidated instead: selection and media actions
+				// dispatch by index, and a stale one would silently
+				// operate on whatever unrelated row occupies it now.
 				_accessibilityFocusedIndex = -1;
 				for (auto i = 0, n = int(elements.size());
 					i < n; ++i) {
@@ -4115,6 +4112,13 @@ void HistoryInner::keyPressEvent(QKeyEvent *e) {
 					}
 				}
 			}
+		} else if (_accessibilityFocusedIndex >= 0) {
+			// A nonnegative index with no cached item means the unread
+			// bar was focused. Follow the bar to wherever it sits now
+			// (rows inserted or removed above shift its index), or
+			// invalidate the focus when the bar is gone: the row that
+			// occupies the old index was never announced to the user.
+			_accessibilityFocusedIndex = accessibilityUnreadBarIndex();
 		}
 		const auto modifiers = e->modifiers()
 			& ~(Qt::KeypadModifier | Qt::GroupSwitchModifier);
@@ -4184,14 +4188,10 @@ void HistoryInner::keyPressEvent(QKeyEvent *e) {
 			const auto elements = accessibleElements();
 			const auto barIndex
 				= accessibilityUnreadBarIndex();
-			const auto elementIndex = (barIndex >= 0
-				&& newIndex > barIndex)
-				? (newIndex - 1)
-				: newIndex;
-			const auto item = (elementIndex >= 0
-				&& elementIndex < int(elements.size()))
-				? elements[elementIndex]->data().get()
-				: nullptr;
+			const auto item = accessibilityItemAtIndex(
+				newIndex,
+				elements,
+				barIndex);
 			if (shiftRange) {
 				extendAccessibilitySelection(
 					_accessibilityFocusedIndex,
@@ -4213,12 +4213,8 @@ void HistoryInner::keyPressEvent(QKeyEvent *e) {
 				}
 			}
 
-			if (_widget->markingMessagesRead()
-				&& (barIndex < 0 || newIndex != barIndex)
-				&& elementIndex >= 0
-				&& elementIndex < int(elements.size())) {
-				session().data().histories().readInboxTill(
-					elements[elementIndex]->data());
+			if (_widget->markingMessagesRead() && item) {
+				session().data().histories().readInboxTill(item);
 			}
 
 			e->accept();
@@ -5968,17 +5964,8 @@ void HistoryInner::extendAccessibilitySelection(
 	// towards it deselects the row being left.
 	const auto elements = accessibleElements();
 	const auto barIndex = accessibilityUnreadBarIndex();
-	const auto itemAt = [&](int index) -> HistoryItem* {
-		if (barIndex >= 0 && index == barIndex) {
-			return nullptr;
-		}
-		const auto elementIndex = (barIndex >= 0 && index > barIndex)
-			? (index - 1)
-			: index;
-		return (elementIndex >= 0
-			&& elementIndex < int(elements.size()))
-			? elements[elementIndex]->data().get()
-			: nullptr;
+	const auto itemAt = [&](int index) {
+		return accessibilityItemAtIndex(index, elements, barIndex);
 	};
 	if (oldIndex < 0) {
 		_accessibilitySelectionAnchor = itemAt(newIndex);
@@ -6024,22 +6011,14 @@ void HistoryInner::extendAccessibilitySelection(
 }
 
 void HistoryInner::playPauseFocusedMedia() {
-	if (_accessibilityFocusedIndex < 0) {
-		return;
-	}
-	const auto barIndex = accessibilityUnreadBarIndex();
-	if (barIndex >= 0 && _accessibilityFocusedIndex == barIndex) {
-		return;
-	}
 	const auto elements = accessibleElements();
-	const auto elementIndex = (barIndex >= 0
-		&& _accessibilityFocusedIndex > barIndex)
-		? (_accessibilityFocusedIndex - 1)
-		: _accessibilityFocusedIndex;
-	if (elementIndex < 0 || elementIndex >= int(elements.size())) {
+	const auto item = accessibilityItemAtIndex(
+		_accessibilityFocusedIndex,
+		elements,
+		accessibilityUnreadBarIndex());
+	if (!item) {
 		return;
 	}
-	const auto item = elements[elementIndex]->data();
 	if (const auto media = item->media()) {
 		if (const auto document = media->document()) {
 			if (document->isVoiceMessage()
@@ -6401,6 +6380,24 @@ int HistoryInner::accessibilityUnreadBarIndex() const {
 	return -1;
 }
 
+HistoryItem *HistoryInner::accessibilityItemAtIndex(
+		int index,
+		const std::vector<Element*> &elements,
+		int barIndex) const {
+	// The unread bar row maps to no item: a focused bar is cached as
+	// a null item with a nonnegative index, so it can never be
+	// mistaken for the message it is anchored to when rows shift.
+	if (index < 0 || (barIndex >= 0 && index == barIndex)) {
+		return nullptr;
+	}
+	const auto elementIndex = (barIndex >= 0 && index > barIndex)
+		? (index - 1)
+		: index;
+	return (elementIndex < int(elements.size()))
+		? elements[elementIndex]->data().get()
+		: nullptr;
+}
+
 int HistoryInner::accessibilityChildCount() const {
 	const auto barIndex = accessibilityUnreadBarIndex();
 	return int(accessibleElements().size()) + (barIndex >= 0 ? 1 : 0);
@@ -6612,6 +6609,13 @@ void HistoryInner::focusInEvent(QFocusEvent *e) {
 			// branch below establishes a fresh focus instead.
 			_accessibilityFocusedItem = nullptr;
 			_accessibilityFocusedIndex = -1;
+		} else if (_accessibilityFocusedIndex >= 0) {
+			// A nonnegative index with no cached item means the unread
+			// bar was focused. Follow the bar to wherever it sits now,
+			// or fall through to pick a fresh focus target when it is
+			// gone: the row that occupies the old index was never
+			// announced to the user.
+			_accessibilityFocusedIndex = accessibilityUnreadBarIndex();
 		}
 		if (_accessibilityFocusedIndex >= 0
 			&& _accessibilityFocusedIndex < count) {
@@ -6623,14 +6627,10 @@ void HistoryInner::focusInEvent(QFocusEvent *e) {
 			? (barIndex + 1)
 			: (count - 1);
 		const auto elements = accessibleElements();
-		const auto elementIndex = (barIndex >= 0
-			&& index > barIndex)
-			? (index - 1)
-			: index;
-		const auto item = (elementIndex >= 0
-			&& elementIndex < int(elements.size()))
-			? elements[elementIndex]->data().get()
-			: nullptr;
+		const auto item = accessibilityItemAtIndex(
+			index,
+			elements,
+			barIndex);
 		setAccessibilityFocusedItem(index, item);
 	});
 }
@@ -6702,13 +6702,7 @@ void HistoryInner::applyAccessibilityFocus(
 		bool announceAlways) {
 	const auto elements = accessibleElements();
 	const auto barIndex = accessibilityUnreadBarIndex();
-	const auto elementIndex = (barIndex >= 0 && index > barIndex)
-		? (index - 1)
-		: index;
-	const auto item = (elementIndex >= 0
-		&& elementIndex < int(elements.size()))
-		? elements[elementIndex]->data().get()
-		: nullptr;
+	const auto item = accessibilityItemAtIndex(index, elements, barIndex);
 	const auto changed = (_accessibilityFocusedIndex != index)
 		|| (_accessibilityFocusedItem != item);
 	_accessibilitySelectionAnchor = nullptr;
@@ -6732,12 +6726,8 @@ void HistoryInner::applyAccessibilityFocus(
 				- (_visibleAreaBottom - _visibleAreaTop));
 		}
 	}
-	if (_widget->markingMessagesRead()
-		&& (barIndex < 0 || index != barIndex)
-		&& elementIndex >= 0
-		&& elementIndex < int(elements.size())) {
-		session().data().histories().readInboxTill(
-			elements[elementIndex]->data());
+	if (_widget->markingMessagesRead() && item) {
+		session().data().histories().readInboxTill(item);
 	}
 }
 

Telegram/SourceFiles/history/history_inner_widget.h

@@ -310,6 +310,10 @@ class HistoryInner
 private:
 	[[nodiscard]] std::vector<Element*> accessibleElements() const;
 	[[nodiscard]] int accessibilityUnreadBarIndex() const;
+	[[nodiscard]] HistoryItem *accessibilityItemAtIndex(
+		int index,
+		const std::vector<Element*> &elements,
+		int barIndex) const;
 	void toggleMessageSelection();
 	void playPauseFocusedMedia();
 	void setAccessibilityFocusedItem(int index, HistoryItem *item);