Compare commits
5 Commits
3b84d5c4ee
...
fix/delay-
| Author | SHA1 | Date | |
|---|---|---|---|
|
55c73ee21a
|
|||
|
5b45b35b3e
|
|||
|
57d79ecf9c
|
|||
|
bfd7064a40
|
|||
|
3f6b8f69fd
|
@@ -1,46 +0,0 @@
|
||||
# fix(chatwin): only fire plugins_post_chat_message_display on incoming
|
||||
|
||||
**Base:** `master` ← `fix/plugin-post-display-incoming-only`
|
||||
|
||||
**Create PR:** https://git.jabber.space/devs/cproof/pulls/new/fix/plugin-post-display-incoming-only
|
||||
|
||||
## 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.
|
||||
|
||||
## 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. If a plugin needs to observe outgoing or history events, it should hook into the corresponding paths instead (`plugins_post_chat_message_send` for outgoing; no current hook for history — could be added separately).
|
||||
14
src/common.c
14
src/common.c
@@ -748,17 +748,17 @@ call_external(gchar** argv)
|
||||
*
|
||||
* This function constructs an argument vector (argv) based on the provided template string, replacing placeholders ("%u" and "%p") with the provided URL and filename, respectively.
|
||||
*
|
||||
* @param template The template string with placeholders.
|
||||
* @param url The URL to replace "%u" (or NULL to skip).
|
||||
* @param filename The filename to replace "%p" (or NULL to skip).
|
||||
* @return The constructed argument vector (argv) as a null-terminated array of strings.
|
||||
* @param template_fmt The template string with placeholders.
|
||||
* @param url The URL to replace "%u" (or NULL to skip).
|
||||
* @param filename The filename to replace "%p" (or NULL to skip).
|
||||
* @return The constructed argument vector (argv) as a null-terminated array of strings.
|
||||
*
|
||||
* @note Remember to free the returned argument vector using `auto_gcharv` or `g_strfreev()`.
|
||||
*/
|
||||
gchar**
|
||||
format_call_external_argv(const char* template, const char* url, const char* filename)
|
||||
format_call_external_argv(const char* template_fmt, const char* url, const char* filename)
|
||||
{
|
||||
gchar** argv = g_strsplit(template, " ", 0);
|
||||
gchar** argv = g_strsplit(template_fmt, " ", 0);
|
||||
|
||||
guint num_args = 0;
|
||||
while (argv[num_args]) {
|
||||
@@ -767,7 +767,7 @@ format_call_external_argv(const char* template, const char* url, const char* fil
|
||||
argv[num_args] = g_strdup(url);
|
||||
} else if (0 == g_strcmp0(argv[num_args], "%p") && filename != NULL) {
|
||||
g_free(argv[num_args]);
|
||||
argv[num_args] = strdup(filename);
|
||||
argv[num_args] = g_strdup(filename);
|
||||
}
|
||||
num_args++;
|
||||
}
|
||||
|
||||
@@ -181,7 +181,7 @@ void get_file_paths_recursive(const char* directory, GSList** contents);
|
||||
gchar* get_random_string(int length);
|
||||
|
||||
gboolean call_external(gchar** argv);
|
||||
gchar** format_call_external_argv(const char* template, const char* url, const char* filename);
|
||||
gchar** format_call_external_argv(const char* template_fmt, const char* url, const char* filename);
|
||||
|
||||
gchar* unique_filename_from_url(const char* url, const char* path);
|
||||
gchar* get_expanded_path(const char* path);
|
||||
|
||||
@@ -10,6 +10,8 @@
|
||||
#ifndef EVENT_COMMON_H
|
||||
#define EVENT_COMMON_H
|
||||
|
||||
#include "glib.h"
|
||||
|
||||
void ev_disconnect_cleanup(void);
|
||||
void ev_inc_connection_counter(void);
|
||||
void ev_reset_connection_counter(void);
|
||||
|
||||
@@ -10,6 +10,8 @@
|
||||
#ifndef PGP_GPG_H
|
||||
#define PGP_GPG_H
|
||||
|
||||
#include "glib.h"
|
||||
|
||||
typedef struct pgp_key_t
|
||||
{
|
||||
gchar* id;
|
||||
|
||||
@@ -10,8 +10,10 @@
|
||||
#ifndef BOOKMARK_IGNORE_H
|
||||
#define BOOKMARK_IGNORE_H
|
||||
|
||||
#include "xmpp/xmpp.h"
|
||||
|
||||
void bookmark_ignore_on_connect(const char* const barejid);
|
||||
void bookmark_ignore_on_disconnect(void);
|
||||
void bookmark_ignore_on_disconnect();
|
||||
gboolean bookmark_ignored(Bookmark* bookmark);
|
||||
gchar** bookmark_ignore_list(gsize* len);
|
||||
void bookmark_ignore_add(const char* const barejid);
|
||||
|
||||
@@ -10,6 +10,8 @@
|
||||
#ifndef UI_TITLEBAR_H
|
||||
#define UI_TITLEBAR_H
|
||||
|
||||
#include "glib.h"
|
||||
|
||||
void create_title_bar(void);
|
||||
void free_title_bar(void);
|
||||
void title_bar_update_virtual(void);
|
||||
|
||||
@@ -43,7 +43,7 @@
|
||||
#include "ai/ai_client.h"
|
||||
|
||||
static const int PAD_MIN_HEIGHT = 100;
|
||||
static const int PAD_THRESHOLD = 3000;
|
||||
static const int PAD_THRESHOLD = 12000; // above buffer cap (~9000): reclaims dead pad space, never fires while scrolling
|
||||
static gboolean _in_redraw = FALSE;
|
||||
|
||||
static void
|
||||
@@ -2172,9 +2172,10 @@ win_redraw(ProfWin* window)
|
||||
unsigned int size = buffer_size(window->layout->buffer);
|
||||
_in_redraw = TRUE;
|
||||
|
||||
// shrink pad back to minimum size and erase it
|
||||
// size the pad to fit the whole buffer (plus headroom) and erase it
|
||||
int cols = getmaxx(window->layout->win);
|
||||
wresize(window->layout->win, PAD_MIN_HEIGHT, cols);
|
||||
int needed = window->layout->buffer->lines + 100;
|
||||
wresize(window->layout->win, MAX(needed, PAD_MIN_HEIGHT), cols);
|
||||
werase(window->layout->win);
|
||||
|
||||
for (unsigned int i = 0; i < size; i++) {
|
||||
|
||||
@@ -10,6 +10,8 @@
|
||||
#ifndef XMPP_BLOCKING_H
|
||||
#define XMPP_BLOCKING_H
|
||||
|
||||
#include <strophe.h>
|
||||
|
||||
void blocking_request(void);
|
||||
int blocked_set_handler(xmpp_stanza_t* stanza);
|
||||
int reporting_set_handler(xmpp_stanza_t* stanza);
|
||||
|
||||
@@ -10,6 +10,8 @@
|
||||
#ifndef XMPP_IQ_H
|
||||
#define XMPP_IQ_H
|
||||
|
||||
#include <strophe.h>
|
||||
|
||||
typedef int (*ProfIqCallback)(xmpp_stanza_t* const stanza, void* const userdata);
|
||||
typedef void (*ProfIqFreeCallback)(void* userdata);
|
||||
|
||||
|
||||
@@ -1132,12 +1132,17 @@ _handle_groupchat(xmpp_stanza_t* const stanza)
|
||||
message->timestamp = NULL;
|
||||
}
|
||||
|
||||
// we want to display the oldest delay
|
||||
message->timestamp = stanza_get_oldest_delay(stanza);
|
||||
|
||||
// now this has nothing to do with MUC history
|
||||
// it's just setting the time to the received time so upon displaying we can use this time
|
||||
// for example in win_println_incoming_muc_msg()
|
||||
// XEP-0203 §4: trust <delay> only from the room or our server, not a client-forged stamp
|
||||
message->timestamp = stanza_get_oldest_delay_from(stanza, from_jid->barejid);
|
||||
if (!message->timestamp && from_jid->domainpart) {
|
||||
message->timestamp = stanza_get_oldest_delay_from(stanza, from_jid->domainpart);
|
||||
}
|
||||
if (!message->timestamp) {
|
||||
const char* my_domain = connection_get_domain();
|
||||
if (my_domain) {
|
||||
message->timestamp = stanza_get_oldest_delay_from(stanza, my_domain);
|
||||
}
|
||||
}
|
||||
if (!message->timestamp) {
|
||||
message->timestamp = g_date_time_new_now_local();
|
||||
}
|
||||
@@ -1268,7 +1273,15 @@ _handle_muc_private_message(xmpp_stanza_t* const stanza)
|
||||
_receive_omemo(stanza, message);
|
||||
#endif
|
||||
|
||||
message->timestamp = stanza_get_delay(stanza);
|
||||
// XEP-0203 §4: trust <delay> only from a server domain, not a client-forged stamp
|
||||
const char* my_domain = connection_get_domain();
|
||||
message->timestamp = stanza_get_delay_from(stanza, my_domain);
|
||||
if (!message->timestamp && message->from_jid->domainpart) {
|
||||
message->timestamp = stanza_get_delay_from(stanza, message->from_jid->domainpart);
|
||||
}
|
||||
if (!message->timestamp) {
|
||||
message->timestamp = g_date_time_new_now_local();
|
||||
}
|
||||
message->body = xmpp_message_get_body(stanza);
|
||||
|
||||
if (!message->plain && !message->body) {
|
||||
@@ -1419,8 +1432,12 @@ _handle_chat(xmpp_stanza_t* const stanza, gboolean is_mam, gboolean is_carbon, c
|
||||
// timestamp provided outside like in a <forwarded> by MAM
|
||||
message->timestamp = timestamp;
|
||||
} else {
|
||||
// timestamp in the message stanza or use time of receival (now)
|
||||
message->timestamp = stanza_get_delay(stanza);
|
||||
// XEP-0203 §4: trust <delay> only from a server domain, not a client-forged stamp
|
||||
const char* my_domain = connection_get_domain();
|
||||
message->timestamp = stanza_get_delay_from(stanza, my_domain);
|
||||
if (!message->timestamp && jid->domainpart) {
|
||||
message->timestamp = stanza_get_delay_from(stanza, jid->domainpart);
|
||||
}
|
||||
if (!message->timestamp) {
|
||||
message->timestamp = g_date_time_new_now_local();
|
||||
}
|
||||
|
||||
@@ -7,6 +7,8 @@
|
||||
* SPDX-License-Identifier: GPL-3.0-or-later WITH OpenSSL-exception
|
||||
*/
|
||||
|
||||
#include "glib.h"
|
||||
|
||||
/*!
|
||||
* \page OX OX Implementation
|
||||
*
|
||||
|
||||
@@ -10,6 +10,9 @@
|
||||
#ifndef XMPP_ROSTER_H
|
||||
#define XMPP_ROSTER_H
|
||||
|
||||
#include "glib.h"
|
||||
#include <strophe.h>
|
||||
|
||||
void roster_request(void);
|
||||
void roster_set_handler(xmpp_stanza_t* const stanza);
|
||||
void roster_result_handler(xmpp_stanza_t* const stanza);
|
||||
|
||||
@@ -1163,7 +1163,7 @@ _stanza_get_delay_timestamp_xep0091(xmpp_stanza_t* const x_stanza)
|
||||
}
|
||||
|
||||
GDateTime*
|
||||
stanza_get_delay_from(xmpp_stanza_t* const stanza, gchar* from)
|
||||
stanza_get_delay_from(xmpp_stanza_t* const stanza, const char* const from)
|
||||
{
|
||||
xmpp_stanza_t* delay = NULL;
|
||||
|
||||
@@ -1195,6 +1195,12 @@ stanza_get_delay_from(xmpp_stanza_t* const stanza, gchar* from)
|
||||
|
||||
GDateTime*
|
||||
stanza_get_oldest_delay(xmpp_stanza_t* const stanza)
|
||||
{
|
||||
return stanza_get_oldest_delay_from(stanza, NULL);
|
||||
}
|
||||
|
||||
GDateTime*
|
||||
stanza_get_oldest_delay_from(xmpp_stanza_t* const stanza, const char* const from)
|
||||
{
|
||||
xmpp_stanza_t* child;
|
||||
const char* child_name;
|
||||
@@ -1205,27 +1211,37 @@ stanza_get_oldest_delay(xmpp_stanza_t* const stanza)
|
||||
child_name = xmpp_stanza_get_name(child);
|
||||
|
||||
if (child_name && g_strcmp0(child_name, STANZA_NAME_DELAY) == 0) {
|
||||
if (from) {
|
||||
const char* child_from = xmpp_stanza_get_attribute(child, STANZA_ATTR_FROM);
|
||||
if (!child_from || g_strcmp0(child_from, from) != 0)
|
||||
continue;
|
||||
}
|
||||
GDateTime* tmp = _stanza_get_delay_timestamp_xep0203(child);
|
||||
|
||||
if (oldest == NULL) {
|
||||
oldest = tmp;
|
||||
} else if (g_date_time_compare(oldest, tmp) == 1) {
|
||||
} else if (tmp && g_date_time_compare(oldest, tmp) == 1) {
|
||||
g_date_time_unref(oldest);
|
||||
oldest = tmp;
|
||||
} else {
|
||||
} else if (tmp) {
|
||||
g_date_time_unref(tmp);
|
||||
}
|
||||
}
|
||||
|
||||
if (child_name && g_strcmp0(child_name, STANZA_NAME_X) == 0) {
|
||||
if (from) {
|
||||
const char* child_from = xmpp_stanza_get_attribute(child, STANZA_ATTR_FROM);
|
||||
if (!child_from || g_strcmp0(child_from, from) != 0)
|
||||
continue;
|
||||
}
|
||||
GDateTime* tmp = _stanza_get_delay_timestamp_xep0091(child);
|
||||
|
||||
if (oldest == NULL) {
|
||||
oldest = tmp;
|
||||
} else if (g_date_time_compare(oldest, tmp) == 1) {
|
||||
} else if (tmp && g_date_time_compare(oldest, tmp) == 1) {
|
||||
g_date_time_unref(oldest);
|
||||
oldest = tmp;
|
||||
} else {
|
||||
} else if (tmp) {
|
||||
g_date_time_unref(tmp);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -305,8 +305,9 @@ xmpp_stanza_t* stanza_create_mediated_invite(xmpp_ctx_t* ctx, const char* const
|
||||
gboolean stanza_contains_chat_state(xmpp_stanza_t* stanza);
|
||||
|
||||
GDateTime* stanza_get_delay(xmpp_stanza_t* const stanza);
|
||||
GDateTime* stanza_get_delay_from(xmpp_stanza_t* const stanza, gchar* from);
|
||||
GDateTime* stanza_get_delay_from(xmpp_stanza_t* const stanza, const char* const from);
|
||||
GDateTime* stanza_get_oldest_delay(xmpp_stanza_t* const stanza);
|
||||
GDateTime* stanza_get_oldest_delay_from(xmpp_stanza_t* const stanza, const char* const from);
|
||||
|
||||
gboolean stanza_is_muc_presence(xmpp_stanza_t* const stanza);
|
||||
gboolean stanza_is_muc_self_presence(xmpp_stanza_t* const stanza,
|
||||
|
||||
@@ -12,6 +12,7 @@
|
||||
|
||||
#include "ui/win_types.h"
|
||||
#include "xmpp/vcard.h"
|
||||
#include <strophe.h>
|
||||
|
||||
vCard* vcard_new();
|
||||
void vcard_free(vCard* vcard);
|
||||
|
||||
Reference in New Issue
Block a user