Ignore reactions and the read status of the blocked users#359
Conversation
WalkthroughThe PR introduces peer-level blocked/filtered checks and reaction-filtering utilities in MessageHelper, then applies this filtering in MessagesController, ReactedHeaderView, ReactedUsersListView, MessageSeenView, and related UI tab counter wiring to exclude blocked/filtered supergroup peers from reaction lists and participant views. ChangesBlocked/Filtered Peer Filtering in Reactions and Participants
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ast-grep (0.42.2)TMessagesProj/src/main/java/org/telegram/ui/ChatActivity.javaThanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 6df3a5db-7bd9-4f29-ae3c-a04c462f8175
📒 Files selected for processing (5)
TMessagesProj/src/main/java/org/telegram/messenger/MessagesController.javaTMessagesProj/src/main/java/org/telegram/ui/Components/ReactedHeaderView.javaTMessagesProj/src/main/java/org/telegram/ui/Components/ReactedUsersListView.javaTMessagesProj/src/main/java/org/telegram/ui/MessageSeenView.javaTMessagesProj/src/main/java/tw/nekomimi/nekogram/helpers/MessageHelper.java
| if (response instanceof TLRPC.TL_messages_messageReactionsList) { | ||
| TLRPC.TL_messages_messageReactionsList list = (TLRPC.TL_messages_messageReactionsList) response; | ||
| int c = list.count; | ||
| int c = Math.max(0, list.count - filterBlockedReactionPeers(list.reactions)); |
There was a problem hiding this comment.
Reaction count is inaccurate — subtracts blocked peers from a 3-item sample, not the total.
getList.limit = 3 at line 234, so list.reactions is capped at 3 items, while list.count is the server's full reaction total (which can be much larger). filterBlockedReactionPeers(list.reactions) therefore returns at most the number of blocked reactors visible in the top-3 sample, not the actual number of blocked reactors on the message. Whenever blocked users have reacted but don't fall within the first 3 returned, the displayed count c still includes them — which defeats this PR's intent for the header count display.
Concrete example: message has 100 reactions including 10 from blocked users, sample contains 0 of them → c = 100, but the intended value is 90.
Possible directions:
- Derive
cfrommessage.messageOwner.reactions.results(aggregated counts) and reconcile against a fully-fetched list of blocked reactors, or - Request a larger window (or paginate) before computing
c, accepting the bandwidth cost, or - Explicitly document that this count is best-effort and the avatar preview is the only filtered view.
| private boolean shouldFilterBlockedPeer(long peerId) { | ||
| return peerId != 0 && message != null && message.isSupergroup() && | ||
| MessageHelper.getInstance(currentAccount).isBlockedOrFilteredPeer(peerId); | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Inspect MessageHelper for existing reaction/blocked-peer filtering APIs
fd -t f 'MessageHelper.java' | xargs -I {} rg -nP -C2 '\b(isBlockedOrFilteredPeer|filterBlockedMessageReactions|shouldFilterBlockedPeer|filterBlockedReactionPeers)\b' {}
# Locate all current duplicates of these helpers across the codebase
rg -nP -C1 '\b(shouldFilterBlockedPeer|filterBlockedReactionPeers)\s*\(' --type=javaRepository: risin42/NagramX
Length of output: 8095
Consider centralizing shouldFilterBlockedPeer and filterBlockedReactionPeers in MessageHelper.
These helpers are duplicated near-verbatim across ReactedUsersListView.java (lines 255–258 and 357–375) and ReactedHeaderView.java (lines 322–344 and 341–343). Since MessageHelper already hosts related centralized utilities (isBlockedOrFilteredPeer, filterBlockedMessageReactions), consolidating these wrappers there would eliminate duplication and prevent drift in the supergroup gate and iteration semantics.
Suggested approach:
// In MessageHelper
public boolean shouldFilterBlockedPeer(MessageObject message, long peerId) { ... }
public int filterBlockedReactionPeers(MessageObject message, List<TLRPC.MessagePeerReaction> reactions) { ... }| public boolean filterBlockedMessageReactions(TLRPC.TL_messageReactions reactions) { | ||
| if (reactions == null || !NekoConfig.ignoreBlocked.Bool()) { | ||
| return false; | ||
| } | ||
| boolean changed = false; | ||
| if (reactions.recent_reactions != null) { | ||
| for (int i = reactions.recent_reactions.size() - 1; i >= 0; i--) { | ||
| TLRPC.MessagePeerReaction reaction = reactions.recent_reactions.get(i); | ||
| if (reaction == null || reaction.peer_id == null) { | ||
| continue; | ||
| } | ||
| long peerId = MessageObject.getPeerId(reaction.peer_id); | ||
| if (peerId != 0 && isBlockedOrFilteredPeer(peerId)) { | ||
| reactions.recent_reactions.remove(i); | ||
| decrementReactionCount(reactions, reaction.reaction); | ||
| changed = true; | ||
| } | ||
| } | ||
| } | ||
| if (reactions.top_reactors != null) { | ||
| for (int i = reactions.top_reactors.size() - 1; i >= 0; i--) { | ||
| TLRPC.MessageReactor reactor = reactions.top_reactors.get(i); | ||
| if (reactor == null || reactor.peer_id == null) { | ||
| continue; | ||
| } | ||
| long peerId = MessageObject.getPeerId(reactor.peer_id); | ||
| if (peerId != 0 && isBlockedOrFilteredPeer(peerId)) { | ||
| reactions.top_reactors.remove(i); | ||
| changed = true; | ||
| } | ||
| } | ||
| } | ||
| return changed; | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for TLRPC.MessageReactor and TLRPC.TL_messageReactions definitions
ast-grep --pattern 'class $_ {
$$$
top_reactors
$$$
}'
# Search for usage patterns where top_reactors and recent_reactions are accessed together
rg -nP -A5 -B5 'top_reactors.*recent_reactions|recent_reactions.*top_reactors'
# Look for comments or documentation about the relationship
rg -nP 'top_reactors' --type=java -C3Repository: risin42/NagramX
Length of output: 14907
🏁 Script executed:
#!/bin/bash
# Find MessageReactor class definition
rg -n "class MessageReactor|class TL_messageReactor" --type=java -A 10
# Find decrementReactionCount method
rg -n "decrementReactionCount" --type=java -A 15
# Check if MessageReactor has a reaction field
rg -n "MessageReactor" --type=java | grep -E "reaction|count" | head -20Repository: risin42/NagramX
Length of output: 8335
🏁 Script executed:
#!/bin/bash
# Find MessagePeerReaction class definition to compare
rg -n "class MessagePeerReaction" --type=java -A 10
# Check how MessageReactor relates to reactions
rg -n "top_reactors" --type=java -B5 -A5 | grep -E "reaction|count" | head -30Repository: risin42/NagramX
Length of output: 5648
Inconsistent reaction count handling when filtering top_reactors.
When removing from recent_reactions (line 1072), the code decrements counts via decrementReactionCount. However, when removing from top_reactors (lines 1077-1089), counts are not decremented despite MessageReactor containing a count field.
The root cause is that MessagePeerReaction includes a reaction field (allowing decrementReactionCount to work), while MessageReactor does not. This means if a blocked peer appears only in top_reactors and not in recent_reactions, their reaction count remains in results even though they are removed from the display, causing count mismatches.
There was a problem hiding this comment.
Actionable comments posted: 2
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: e00a47cb-23ce-43aa-950e-a36796ea5823
📒 Files selected for processing (3)
TMessagesProj/src/main/java/org/telegram/ui/ChatActivity.javaTMessagesProj/src/main/java/org/telegram/ui/Components/ReactedUsersListView.javaTMessagesProj/src/main/java/org/telegram/ui/Components/ReactionTabHolderView.java
| final ReactionTabHolderView tabHolder = (ReactionTabHolderView) tabsView.getChildAt(position); | ||
| v.setOnCountChangedListener((view, filteredCount) -> { | ||
| if (tabIndex < 0) { | ||
| tabHolder.setCounter(filteredCount); | ||
| } else { | ||
| tabHolder.setCounter(currentAccount, counters.get(tabIndex), filteredCount); | ||
| } | ||
| }); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Check listener cleanup and view recycling patterns in ReactedUsersListView
# Search for setOnCountChangedListener to understand the listener lifecycle
rg -nP --type=java -C5 'setOnCountChangedListener' -g '!**/test/**'
# Check if there's listener cleanup (removeListener, clear, null assignment)
rg -nP --type=java -C3 '\b(remove.*[Ll]istener|clear.*[Ll]istener|OnCountChangedListener.*=\s*null)' -g '!**/test/**' -g '**/ReactedUsersListView.java'
# Check how cachedViews are managed (cleanup, recycling)
rg -nP --type=java -C5 'cachedViews\.(clear|remove)' -g '**/ChatActivity.java'Repository: risin42/NagramX
Length of output: 2984
🏁 Script executed:
#!/bin/bash
# Get the full method context around lines 31985-31993
echo "=== Full method context (lines 31970-32010) ==="
sed -n '31970,32010p' TMessagesProj/src/main/java/org/telegram/ui/ChatActivity.java
# Check what tabsView is and how it's used
echo -e "\n=== Search for tabsView declaration and usage ==="
rg -n 'tabsView\s*=' TMessagesProj/src/main/java/org/telegram/ui/ChatActivity.java | head -20
# Check ReactedUsersListView to see how/when the listener callback is invoked
echo -e "\n=== ReactedUsersListView callback invocation ==="
rg -n -A3 'onCountChangedListener' TMessagesProj/src/main/java/org/telegram/ui/Components/ReactedUsersListView.java | grep -A3 'onCountChangedListener\.'Repository: risin42/NagramX
Length of output: 3310
🏁 Script executed:
#!/bin/bash
# Check how ReactedUsersListView triggers the count changed callback
echo "=== When does onCountChangedListener fire in ReactedUsersListView? ==="
rg -n -B10 'onCountChangedListener.onCountChanged' TMessagesProj/src/main/java/org/telegram/ui/Components/ReactedUsersListView.java | head -40
# Check if there's any cleanup of listeners when views are recycled
echo -e "\n=== View recycling and cleanup in ChatActivity adapter ==="
sed -n '31940,32010p' TMessagesProj/src/main/java/org/telegram/ui/ChatActivity.java
# Look for cachedViews.get to see if views are being reused
echo -e "\n=== How cachedViews is used - view reuse pattern ==="
rg -n -B3 -A3 'cachedViews\.get' TMessagesProj/src/main/java/org/telegram/ui/ChatActivity.java | head -50Repository: risin42/NagramX
Length of output: 6612
🏁 Script executed:
#!/bin/bash
# Check if tabsView children are ever modified after initial setup
echo "=== Checking if tabsView is modified after creation ==="
rg -n 'tabsView\.(add|remove|insert|clear|setText)' TMessagesProj/src/main/java/org/telegram/ui/ChatActivity.java
# Check the OnPageChangeListener to see if tabs might change during paging
echo -e "\n=== OnPageChangeListener implementation context ==="
sed -n '32005,32050p' TMessagesProj/src/main/java/org/telegram/ui/ChatActivity.java
# Check if there are any conditions where position could be invalid or tabs reorganized
echo -e "\n=== Context after line 32010 - view management ==="
sed -n '32010,32080p' TMessagesProj/src/main/java/org/telegram/ui/ChatActivity.javaRepository: risin42/NagramX
Length of output: 8775
Capture of view reference in asynchronously-firing listener creates stale reference risk.
The tabHolder is captured from tabsView.getChildAt(position) before the listener is set, but the listener fires asynchronously (via network response in ReactedUsersListView lines 284-292). Combined with view caching in the ViewPager adapter:
- ReactedUsersListView stores the listener as a field, keeping it alive indefinitely
- When the network response arrives and the callback fires, it uses the captured
tabHolderreference from creation time - If the view hierarchy has changed or the view instance has been recycled, counter updates apply to the wrong tab
- This pattern likely causes the "reaction num display err" mentioned in the commit message
Safer approach: Look up the view inside the lambda when the callback actually fires, rather than capturing the reference beforehand.
Suggested fix
-final int tabIndex = index;
-final ReactionTabHolderView tabHolder = (ReactionTabHolderView) tabsView.getChildAt(position);
-v.setOnCountChangedListener((view, filteredCount) -> {
- if (tabIndex < 0) {
- tabHolder.setCounter(filteredCount);
- } else {
- tabHolder.setCounter(currentAccount, counters.get(tabIndex), filteredCount);
- }
-});
+final int tabIndex = index;
+final int tabPosition = position;
+v.setOnCountChangedListener((view, filteredCount) -> {
+ ReactionTabHolderView tabHolder = (ReactionTabHolderView) tabsView.getChildAt(tabPosition);
+ if (tabHolder != null) {
+ if (tabIndex < 0) {
+ tabHolder.setCounter(filteredCount);
+ } else {
+ tabHolder.setCounter(currentAccount, counters.get(tabIndex), filteredCount);
+ }
+ }
+});| int filteredCount = Math.max(0, res.count - filterBlockedReactionPeers(res.reactions)); | ||
| if (onCountChangedListener != null) { | ||
| onCountChangedListener.onCountChanged(this, filteredCount); | ||
| } |
There was a problem hiding this comment.
Use cumulative blocked-removal when reporting filtered total.
filteredCount subtracts only the blocked peers removed from the current page, so paginated loads can report inflated/unstable tab counts.
Suggested fix
@@
- private boolean showReactionPreview;
+ private boolean showReactionPreview;
+ private int filteredRemovedTotal;
@@
private void load() {
isLoading = true;
+ if (offset == null) {
+ filteredRemovedTotal = 0;
+ }
@@
- int filteredCount = Math.max(0, res.count - filterBlockedReactionPeers(res.reactions));
+ filteredRemovedTotal += filterBlockedReactionPeers(res.reactions);
+ int filteredCount = Math.max(0, res.count - filteredRemovedTotal);
if (onCountChangedListener != null) {
onCountChangedListener.onCountChanged(this, filteredCount);
}
|
Fixed in 1a6ee22 |
2 participants
忽略掉被屏蔽用户的已读状态和点击reaction
Summary by CodeRabbit
Bug Fixes
New Features