Improve message filtering#371
Conversation
…iding (also option to only mask matched word)
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughSummary by CodeRabbitRelease Notes
WalkthroughThis PR adds Ayu spoiler masking (actions: hide, spoiler-all, spoiler-match) with per-filter colors, integrates it into message layout/rendering and hit-testing, updates AyuFilter to action semantics, and adds UI/config for editing filters, masked blocked-user settings, and per-user color/action controls. ChangesAyu Spoiler Masking Implementation
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 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.3)TMessagesProj/src/main/java/org/telegram/messenger/MessageObject.javaTMessagesProj/src/main/java/org/telegram/ui/Cells/ChatMessageCell.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: 9
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 9a4744a7-52b2-46cf-9fbb-7cdb46ca2322
📒 Files selected for processing (9)
TMessagesProj/src/main/java/org/telegram/messenger/MessageObject.javaTMessagesProj/src/main/java/org/telegram/ui/Cells/ChatMessageCell.javaTMessagesProj/src/main/java/org/telegram/ui/ChatActivity.javaTMessagesProj/src/main/java/tw/nekomimi/nekogram/filters/AyuFilter.javaTMessagesProj/src/main/java/tw/nekomimi/nekogram/filters/RegexFilterEditActivity.javaTMessagesProj/src/main/java/tw/nekomimi/nekogram/filters/RegexFiltersSettingActivity.javaTMessagesProj/src/main/java/tw/nekomimi/nekogram/filters/ShadowBanListActivity.javaTMessagesProj/src/main/kotlin/xyz/nextalone/nagram/NaConfig.ktTMessagesProj/src/main/res/values/strings_na.xml
|
hmm, i will work on those suggestion when i get time |
1. make ayu media and text spoiler mask independent from telegram native spoiler reveal setting 2. make sure AnimatedEmojiSpans are also handled and masked 3. move ayuClipPath to class field and rewind instead of allocating new Path each draw frame 4. fix cache issue of blocked user after caching 5. make entire spoiler color row clickable for accessibility 6. change default blocked user mask color from white to gray for visibility on light backgrounds
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
TMessagesProj/src/main/java/org/telegram/messenger/MessageObject.java (1)
8391-8416:⚠️ Potential issue | 🟠 Major | ⚡ Quick winMask Telegram spoiler emoji in the shared patched layout too.
This patched layout is reused for both Ayu masks and native Telegram spoilers, but only the Ayu branch strips self-drawing emoji spans. If a block contains both spoiler types, animated/custom emoji inside the Telegram spoiler range can still draw through because the Telegram branch only adds a transparent foreground span.
💡 Suggested fix
if (block.textLayout.getText() instanceof Spanned) { Spanned sp = (Spanned) block.textLayout.getText(); TextStyleSpan[] tgSpans = sp.getSpans(0, sp.length(), TextStyleSpan.class); for (TextStyleSpan span : tgSpans) { if (span.isSpoiler()) { int start = sp.getSpanStart(span); int end = sp.getSpanEnd(span); if (start >= 0 && end > start && end <= sb.length()) { + for (Emoji.EmojiSpan e : sb.getSpans(start, end, Emoji.EmojiSpan.class)) { + final Emoji.EmojiSpan captured = e; + sb.setSpan(new ReplacementSpan() { + `@Override` + public int getSize(`@NonNull` Paint paint, CharSequence text, int s, int e2, `@Nullable` Paint.FontMetricsInt fm) { + return captured.getSize(paint, text, s, e2, fm); + } + `@Override` + public void draw(`@NonNull` Canvas canvas, CharSequence text, int s, int e2, float x, int top, int y, int bottom, `@NonNull` Paint paint) { + } + }, sb.getSpanStart(e), sb.getSpanEnd(e), Spanned.SPAN_EXCLUSIVE_EXCLUSIVE); + sb.removeSpan(e); + } + for (AnimatedEmojiSpan e : sb.getSpans(start, end, AnimatedEmojiSpan.class)) { + final AnimatedEmojiSpan captured = e; + sb.setSpan(new ReplacementSpan() { + `@Override` + public int getSize(`@NonNull` Paint paint, CharSequence text, int s, int e2, `@Nullable` Paint.FontMetricsInt fm) { + return captured.getSize(paint, text, s, e2, fm); + } + `@Override` + public void draw(`@NonNull` Canvas canvas, CharSequence text, int s, int e2, float x, int top, int y, int bottom, `@NonNull` Paint paint) { + } + }, sb.getSpanStart(e), sb.getSpanEnd(e), Spanned.SPAN_EXCLUSIVE_EXCLUSIVE); + sb.removeSpan(e); + } sb.setSpan(new ForegroundColorSpan(Color.TRANSPARENT), start, end, Spanned.SPAN_EXCLUSIVE_EXCLUSIVE); } } } }
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 94aae10a-4811-4400-b225-5a0fc0e222ec
📒 Files selected for processing (5)
TMessagesProj/src/main/java/org/telegram/messenger/MessageObject.javaTMessagesProj/src/main/java/org/telegram/ui/Cells/ChatMessageCell.javaTMessagesProj/src/main/java/tw/nekomimi/nekogram/filters/AyuFilter.javaTMessagesProj/src/main/java/tw/nekomimi/nekogram/filters/RegexFilterEditActivity.javaTMessagesProj/src/main/kotlin/xyz/nextalone/nagram/NaConfig.kt
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 5b01b4f3-c1cf-445a-9889-c9ece3e5b108
📒 Files selected for processing (1)
TMessagesProj/src/main/java/org/telegram/messenger/MessageObject.java
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
TMessagesProj/src/main/java/org/telegram/messenger/MessageObject.java (3)
8411-8417:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winDefine
suppressSelfDrawingSpans(...)before calling it.Line 8416 invokes
suppressSelfDrawingSpans(sb, start, end), but this helper is not declared anywhere inMessageObject.java. As written, this file will not compile.
271-273:⚠️ Potential issue | 🟠 Major | ⚡ Quick winPreserve
ayuSpoilerRevealedacrossMessageObjectreplacement.The new reveal flag is transient, but unlike
isSpoilersRevealedit is never copied incopyStableParams(). Any refresh that swaps in a newMessageObjectwill re-mask content the user already revealed.💡 Suggested fix
isSpoilersRevealed = old.isSpoilersRevealed; + ayuSpoilerRevealed = old.ayuSpoilerRevealed; messageOwner.replyStory = old.messageOwner.replyStory; if (messageOwner.media != null && old.messageOwner.media != null) { messageOwner.media.storyItem = old.messageOwner.media.storyItem; } if (isSpoilersRevealed && textLayoutBlocks != null) { for (TextLayoutBlock block : textLayoutBlocks) { block.spoilers.clear(); } } + if (ayuSpoilerRevealed && textLayoutBlocks != null) { + for (TextLayoutBlock block : textLayoutBlocks) { + block.ayuSpoilerGroups = null; + block.ayuSpoilersPatchedTextLayout.set(null); + } + }
8434-8434:⚠️ Potential issue | 🟠 Major | ⚡ Quick winFix Ayu patched spoiler layout caching to use
ayuSpoilersPatchedTextLayout
MessageObject.ayuPreBuildPatchedLayout()stores intoblock.spoilersPatchedTextLayout(8434), never intoblock.ayuSpoilersPatchedTextLayout(declared but not written).ChatMessageCellrenders Ayu spoilers usingblock.spoilersPatchedTextLayout(e.g., ~17230), soblock.ayuSpoilersPatchedTextLayoutis effectively unused.- Ayu spoiler invalidation clears only
block.spoilersPatchedTextLayout(8857-8859, 9319-9322) andblock.ayuSpoilerGroups, leavingblock.ayuSpoilersPatchedTextLayoutstale/not cleared.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 0c0a1b82-8733-4a61-a31b-3bc428d3bd37
📒 Files selected for processing (1)
TMessagesProj/src/main/java/org/telegram/messenger/MessageObject.java
|
this coderabbit ... |
…fix Ayu patched spoiler layout caching)
|
@coderabbitai Do a proper review for once ok, include all suggestions at once if there is any left |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
gh pr view 371 --repo risin42/NagramX --json files,number,titleLength of output: 1408 🏁 Script executed: #!/bin/bash
gh pr diff 371 --repo risin42/NagramX 2>/dev/null | head -4000Length of output: 88389
🐛 Critical / Correctness1.
|
| # | Severity | File | Issue |
|---|---|---|---|
| 1 | 🔴 Critical | ChatMessageCell.java |
clipOutPath crashes API < 26 |
| 2 | 🔴 Critical | AyuFilter.java |
getMatchingFilterModels uncached regex scan per render |
| 3 | 🟠 Bug | ChatMessageCell.java |
Early return without generateLayout leaves stale layout |
| 4 | 🟠 Bug | RegexFilterEditActivity.java |
Stored spoiler color silently reset on hide-action edit |
| 5 | 🟡 Perf | ChatMessageCell.java |
applyAyuFilterSpoiler + generateLayout on every bind |
| 6 | 🟡 Perf | MessageObject.java |
O(spans × text) allocation in ayuBuildSpoilerGroups |
| 7 | 🟡 Design | AyuFilter.java |
Magic -1 return value needs named constant |
| 8 | 🟡 Logic | AyuFilter.java |
Shadow-ban hide vs blocked-peer mask priority edge case |
| 9 | 🔵 Minor | strings_na.xml |
Missing space in string |
| 10 | 🔵 Minor | Multiple | Import ordering |
| 11 | 🔵 Minor | MessageObject.java |
Redundant null check |
| 12 | 🔵 Minor | ChatMessageCell.java |
Duplicate hit-test loops (self-acknowledged) |
| 13 | 🔵 Minor | Multiple | PipetteDelegate no-op boilerplate |
The most important fixes to land before merge are #1 (crashes on API < 26), #2 (regex perf), and #3 (stale layout). Take your time on the rest.
(ง •̀_•́)ง
|
Thanks for your work, but I do not plan to add a masking option to the message filters. And bro...No AI Slop, thanks. |
2 participants
What is this?
This PR implement a masking feature for message filters, before user can hide messages completely that matches regex flters but now they can mask them with spoiler tag. Not only that but with a different spoiler tag color to identify what filter is hiding it and also know that its not normal telegram spoiler tag.
Additionally, instead of hiding blocked users messages. User can choose to mask them. Similarly for shadow ban list, they can manually change hide to mask and select a color of masking.
Why this?
I have been trying to filter out seeing some particular kind of text but i didnt wanted to hide whole message (what if a word is mentioned in context and filter hides it). So for my use case i worked on this feature
Note
I am not sure how good the code quality is, i am not yet familiar with this codebase and i didn't work on much of android stuff (i did learn basic of android app in past but not much). So for understanding files and getting help with this feature implementation i used chatbot. But i still added some redundant code block (in
checkSpoilerMotionEventmethod)Also, sorry if PR is too messy to review (i should have committed step by step)