fix/plugin-post-display-incoming-only #132

Manually merged

jabber.developer2 merged 1 commits from fix/plugin-post-display-incoming-only into master 2026-06-01 13:52:40 +00:00

Conversation 0 Commits 1 Files Changed 1 -5

jabber.developer2 commented 2026-06-01 13:35:19 +00:00

Collaborator

Copy Link

Problem

Plugins listening on plugins_post_chat_message_display (e.g. sounds.py — plays a notification sound on a new incoming chat message) were firing in places that are not "a remote party sent a chat message and it was displayed":

• Opening a chat — every message loaded from history triggered the hook, so the user heard the notification sound once per historical message.
• Sending a chat message — the outgoing-message path triggered the hook, so sending played the incoming sound.
• Receiving a carbon copy of your own outgoing message from another device — same, played the incoming sound.

Net effect for sounds.py (and any plugin that treats the hook as "a new message arrived"): notification sound on every open / send / carbon, which is essentially constant nuisance.

Root cause

The post-display hook was invoked from four sites in src/ui/chatwin.c:

Function	Path
chatwin_incoming_msg	remote → us (true incoming)
chatwin_outgoing_msg	us → remote
chatwin_outgoing_carbon	us → remote, mirrored back via XEP-0280 carbons
_chatwin_history	history scroll-back
chatwin_db_history	DB-backed history load

Only the first matches the hook's semantic. The other four were spurious — they treated "display happened" as the trigger condition instead of "an incoming message was displayed".

Fix

Remove the plugins_post_chat_message_display(...) call from the four spurious sites. Keep it only in chatwin_incoming_msg. The corresponding pre-display hook (plugins_pre_chat_message_display) and the msg->plain = old_plain swap that bracket it stay in place — those affect how the message is rendered, independent of the post hook.

One file, five deletions.

Why pre stays asymmetric to post

After this fix the two hooks no longer pair up in outgoing / carbon / history paths: pre_chat_message_display is still called there, post_chat_message_display is not. That asymmetry is deliberate. The two hooks have different semantics:

• pre_chat_message_display lets a plugin modify the text that will be rendered (formatting, redaction, link rewriting, etc.). This is useful in every display path and is kept everywhere.
• post_chat_message_display is a notification that an incoming chat message arrived and was displayed. It is the trigger plugins like sounds.py legitimately listen to. It should not fire on history or on the user's own outgoing.

Removing pre together with post would lose display-mutation capability for everything except true incoming, which is unrelated to the bug.

Verify

• Open a chat that has stored history → expect zero notification sounds from sounds.py during the history render.
• Send a message → expect zero notification sound for your own outgoing.
• Receive an outgoing carbon (send from another XMPP client logged into the same account) → expect zero notification sound.
• Receive an actual incoming message from the remote contact → expect the notification sound exactly once.

Note for plugin authors

Plugins relying on the previous (broader) trigger semantics will see the hook only on true incoming displays. Replacements:

• Outgoing → plugins_post_chat_message_send (already fires from src/event/client_events.c:176).
• History → no replacement; a historical message is not an event the plugin should treat as a new arrival. Could be added separately if a legitimate use-case appears.
• Outgoing carbons (XEP-0280, your own message echoed back from another XMPP client of yours) — no replacement hook exists today. Plugins that want to observe these will need a new hook (e.g. plugins_post_chat_message_send_carbon); intentionally out of scope here.

Checked for collateral

• MUC equivalent (plugins_post_room_message_display) — searched src/ui/mucwin.c and src/event/client_events.c; the analogous mis-use does not exist for groupchat. Only plugins_post_room_message_send is wired, on the outgoing path. No companion MUC fix needed.
• MAM (Message Archive) replay — goes through chatwin_db_history. After this fix the post-display hook does not fire for MAM-loaded messages, which is the desired behaviour (archived messages are not new arrivals).
• Memory / lifetime of pre swap — the auto_char plugin string is freed at the end of its declaration scope, after msg->plain = old_plain has restored the original pointer. No use-after-free introduced.
• Receipts / chat states / encryption hooks — separate plugin hooks, not touched.

## Problem Plugins listening on `plugins_post_chat_message_display` (e.g. `sounds.py` — plays a notification sound on a new incoming chat message) were firing in places that are not "a remote party sent a chat message and it was displayed": - Opening a chat — every message loaded from history triggered the hook, so the user heard the notification sound once per historical message. - Sending a chat message — the outgoing-message path triggered the hook, so sending played the incoming sound. - Receiving a carbon copy of your own outgoing message from another device — same, played the incoming sound. Net effect for `sounds.py` (and any plugin that treats the hook as "a new message arrived"): notification sound on every open / send / carbon, which is essentially constant nuisance. ## Root cause The post-display hook was invoked from four sites in `src/ui/chatwin.c`: | Function | Path | |---|---| | `chatwin_incoming_msg` | remote → us (true incoming) | | `chatwin_outgoing_msg` | us → remote | | `chatwin_outgoing_carbon` | us → remote, mirrored back via XEP-0280 carbons | | `_chatwin_history` | history scroll-back | | `chatwin_db_history` | DB-backed history load | Only the first matches the hook's semantic. The other four were spurious — they treated "display happened" as the trigger condition instead of "an incoming message was displayed". ## Fix Remove the `plugins_post_chat_message_display(...)` call from the four spurious sites. Keep it only in `chatwin_incoming_msg`. The corresponding pre-display hook (`plugins_pre_chat_message_display`) and the `msg->plain = old_plain` swap that bracket it stay in place — those affect how the message is rendered, independent of the post hook. One file, five deletions. ### Why `pre` stays asymmetric to `post` After this fix the two hooks no longer pair up in outgoing / carbon / history paths: `pre_chat_message_display` is still called there, `post_chat_message_display` is not. That asymmetry is deliberate. The two hooks have different semantics: - `pre_chat_message_display` lets a plugin **modify the text that will be rendered** (formatting, redaction, link rewriting, etc.). This is useful in every display path and is kept everywhere. - `post_chat_message_display` is a **notification that an incoming chat message arrived and was displayed**. It is the trigger plugins like `sounds.py` legitimately listen to. It should not fire on history or on the user's own outgoing. Removing `pre` together with `post` would lose display-mutation capability for everything except true incoming, which is unrelated to the bug. ## Verify - Open a chat that has stored history → expect zero notification sounds from `sounds.py` during the history render. - Send a message → expect zero notification sound for your own outgoing. - Receive an outgoing carbon (send from another XMPP client logged into the same account) → expect zero notification sound. - Receive an actual incoming message from the remote contact → expect the notification sound exactly once. ## Note for plugin authors Plugins relying on the previous (broader) trigger semantics will see the hook only on true incoming displays. Replacements: - **Outgoing** → `plugins_post_chat_message_send` (already fires from `src/event/client_events.c:176`). - **History** → no replacement; a historical message is not an event the plugin should treat as a new arrival. Could be added separately if a legitimate use-case appears. - **Outgoing carbons** (XEP-0280, your own message echoed back from another XMPP client of yours) — no replacement hook exists today. Plugins that want to observe these will need a new hook (e.g. `plugins_post_chat_message_send_carbon`); intentionally out of scope here. ## Checked for collateral - **MUC equivalent (`plugins_post_room_message_display`)** — searched `src/ui/mucwin.c` and `src/event/client_events.c`; the analogous mis-use does not exist for groupchat. Only `plugins_post_room_message_send` is wired, on the outgoing path. No companion MUC fix needed. - **MAM (Message Archive) replay** — goes through `chatwin_db_history`. After this fix the post-display hook does not fire for MAM-loaded messages, which is the desired behaviour (archived messages are not new arrivals). - **Memory / lifetime of `pre` swap** — the `auto_char` plugin string is freed at the end of its declaration scope, after `msg->plain = old_plain` has restored the original pointer. No use-after-free introduced. - **Receipts / chat states / encryption hooks** — separate plugin hooks, not touched.

jabber.developer2 added 2 commits 2026-06-01 13:35:20 +00:00

fix(chatwin): only fire plugins_post_chat_message_display on incoming ... a04a8948e1

The trigger was wired into outgoing / outgoing-carbon / history paths
too, which made plugins like sounds.py play the "new message" sound
when opening a chat (history loads), sending a message yourself, or
receiving a carbon of your own sent message from another device.

Restrict the call to chatwin_incoming_msg — matches the semantic of
"a remote party sent a chat message and it was displayed".

docs: PR description for chatwin trigger fix 3b84d5c4ee

jabber.developer2 force-pushed fix/plugin-post-display-incoming-only from 3b84d5c4ee to a04a8948e1 2026-06-01 13:37:43 +00:00 Compare

jabber.developer2 manually merged commit a04a8948e1 into master 2026-06-01 13:52:40 +00:00

Sign in to join this conversation.

Reviewers

No Reviewers

Labels

Clear labels

Compat/Breaking

Breaking change that won't be backward compatible

Kind/Bug

Something is not working

Kind/Documentation

Documentation changes

Kind/Enhancement

Improve existing functionality

Kind/Feature

New functionality

Kind/Security

This is security issue

Kind/Testing

Issue or pull request related to testing

Priority

Critical

1

The priority is critical

Priority

High

2

The priority is high

Priority

Low

4

The priority is low

Priority

Medium

3

The priority is medium

Reviewed

Confirmed

1

Issue has been confirmed

Reviewed

Duplicate

2

This issue or pull request already exists

Reviewed

Invalid

3

Invalid issue

Reviewed

Won't Fix

3

This issue won't be fixed

Status

Abandoned

3

Somebody has started to work on this but abandoned work

Status

Blocked

1

Something is blocking this issue or pull request

Status

Need More Info

2

Feedback is required to reproduce issue or to continue work

No Label

Milestone

No items

No Milestone

Projects

Clear projects

No project

Assignees

Clear assignees

admin jabber.developer jabber.developer2

No Assignees

1 Participants

Notifications

Subscribe

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: devs/cproof#132

No description provided.