From 4ded6280915083be048ac6185ed143bdf14cf3ed Mon Sep 17 00:00:00 2001 From: Konstantin Obenland Date: Tue, 10 Nov 2026 12:39:51 -0600 Subject: [PATCH 1/2] Trac: Slack: Don't announce attachment notifications as new tickets Attachment notifications carry the plain ticket URL, so the hook looked for an attachment change line in the raw body to tell them apart from new tickets. That line is missed when Trac wraps a long file name onto the next line, or when the body is encoded, and the email was then posted as a new ticket. Check the In-Reply-To header instead, which Trac sets on every ticket notification except the one for a new ticket. The comment handler now also joins a wrapped attachment line, so it is posted as one attachment change with the full file name. Fixes #7720. Co-Authored-By: Claude Opus 5.5 --- common/includes/slack/trac/comment-handler.php | 12 +++++++++++- .../tests/slack/trac/comment-handler.php | 16 ++++++++++++++-- svn.wordpress.org/bin/slack-trac-hook.php | 14 +++++++++++--- 3 files changed, 36 insertions(+), 6 deletions(-) diff --git a/common/includes/slack/trac/comment-handler.php b/common/includes/slack/trac/comment-handler.php index ae47f599eb..af6795b5d0 100644 --- a/common/includes/slack/trac/comment-handler.php +++ b/common/includes/slack/trac/comment-handler.php @@ -242,7 +242,17 @@ function process_message() { while ( $lines && '' !== current( $lines ) ) { $line = array_shift( $lines ); - if ( preg_match( '~Attachment "([^"]+)" (added|removed)\.$~', $line, $matches ) ) { // * Attachment "test.txt" added/removed. + // Trac wraps long file names onto indented lines. + if ( 0 === strpos( $line, ' * Attachment "' ) ) { + while ( $lines && '' !== current( $lines ) && 0 !== strpos( current( $lines ), ' * ' ) ) { + if ( '-' !== substr( $line, -1 ) ) { + $line .= ' '; + } + $line .= ltrim( array_shift( $lines ) ); + } + } + + if ( preg_match( '~Attachment "(.+)" (added|removed)\.$~', $line, $matches ) ) { // * Attachment "test.txt" added/removed. $changes[] = "_*attachment:*_ `{$matches[1]}` {$matches[2]}"; } else { if ( 0 === strpos( $line, ' * keywords:' ) ) { diff --git a/common/includes/tests/slack/trac/comment-handler.php b/common/includes/tests/slack/trac/comment-handler.php index 9cf7faf8a0..57918a6e3f 100644 --- a/common/includes/tests/slack/trac/comment-handler.php +++ b/common/includes/tests/slack/trac/comment-handler.php @@ -67,7 +67,7 @@ public function test_last_line_is_kept( array $body, string $comment, array $cha */ public function data_last_line_is_kept(): array { return array( - 'comment' => array( + 'comment' => array( array( '', 'Comment (by obenland):', @@ -79,7 +79,7 @@ public function data_last_line_is_kept(): array { "I started such a thread.\n\nSo it was closed.", array(), ), - 'changes only' => array( + 'changes only' => array( array( 'Changes (by obenland):', '', @@ -92,6 +92,18 @@ public function data_last_line_is_kept(): array { '_*status:*_ new => assigned', ), ), + 'wrapped attachment' => array( + array( + 'Changes (by adamsilverstein):', + '', + ' * Attachment "Monosnap Site Health - Info ‹ My... Website — WordPress', + ' 2024-07-15 15-14-25.jpg" added.', + ), + '', + array( + '_*attachment:*_ `Monosnap Site Health - Info ‹ My... Website — WordPress 2024-07-15 15-14-25.jpg` added', + ), + ), ); } } diff --git a/svn.wordpress.org/bin/slack-trac-hook.php b/svn.wordpress.org/bin/slack-trac-hook.php index 1493e5c92b..761677fa79 100644 --- a/svn.wordpress.org/bin/slack-trac-hook.php +++ b/svn.wordpress.org/bin/slack-trac-hook.php @@ -23,9 +23,17 @@ $ticket_url = $matches[2]; $ticket_base_url = $matches[3]; $ticket = $matches[4]; - // Attachment notifcations use the ticket URL, check for attachment change line and process mail as comment on match. - $is_attachment = (bool) preg_grep( '/^ \* Attachment "([^"]+)"/', $lines ); - $type = ( isset( $matches[5] ) || $is_attachment ) ? 'comment' : 'ticket'; + // Attachment notifications use the ticket URL, but Trac sets In-Reply-To on all except new-ticket ones. + $is_reply = false; + foreach ( $lines as $header ) { + if ( '' === trim( $header ) ) { + break; + } + if ( 0 === stripos( $header, 'In-Reply-To:' ) ) { + $is_reply = true; + } + } + $type = ( isset( $matches[5] ) || $is_reply ) ? 'comment' : 'ticket'; // phpcs:ignore WordPress.WP.GlobalVariablesOverride.Prohibited -- WordPress isn't loaded here. preg_match( '~^https?://([^.]+)\.trac~i', $ticket_base_url, $matches ); $trac = $matches[1]; break; From 319d3f3837d3703a98a255378da01a3e28de0fea Mon Sep 17 00:00:00 2001 From: Konstantin Obenland Date: Tue, 10 Nov 2026 13:32:46 -0600 Subject: [PATCH 2/2] Trac: Slack: Don't add a space inside wrapped CJK attachment names Trac's text wrapper breaks runs of CJK characters between any two characters, so joining those wrapped lines with a space put a space into the file name that wasn't there. Skip the space when both sides of the break are characters Trac treats as breakable. Co-Authored-By: Claude Opus 5.5 --- common/includes/slack/trac/comment-handler.php | 16 ++++++++++++++-- .../tests/slack/trac/comment-handler.php | 18 +++++++++++++++--- 2 files changed, 29 insertions(+), 5 deletions(-) diff --git a/common/includes/slack/trac/comment-handler.php b/common/includes/slack/trac/comment-handler.php index af6795b5d0..86ee3b4ce1 100644 --- a/common/includes/slack/trac/comment-handler.php +++ b/common/includes/slack/trac/comment-handler.php @@ -23,6 +23,13 @@ class Comment_Handler { */ protected $keywords_added = array(); + /** + * Characters Trac's text wrapper breaks between, mirroring UnicodeTextWrapper::breakable_char_ranges. + * + * @var string + */ + const BREAKABLE_CHARS = '[\x{1100}-\x{11FF}\x{2E80}-\x{2EFF}\x{3000}-\x{30FF}\x{3130}-\x{319F}\x{31C0}-\x{31EF}\x{3200}-\x{4DBF}\x{4E00}-\x{9FFF}\x{A960}-\x{A97F}\x{AC00}-\x{D7FF}\x{F900}-\x{FAFF}\x{FE30}-\x{FE4F}\x{FF00}-\x{FFEF}\x{20000}-\x{3FFFF}]'; + function __construct( \Dotorg\Slack\Send $send, array $email_message ) { $this->send = $send; $this->lines = $email_message; @@ -245,10 +252,15 @@ function process_message() { // Trac wraps long file names onto indented lines. if ( 0 === strpos( $line, ' * Attachment "' ) ) { while ( $lines && '' !== current( $lines ) && 0 !== strpos( current( $lines ), ' * ' ) ) { - if ( '-' !== substr( $line, -1 ) ) { + $next_line = ltrim( array_shift( $lines ) ); + + // Trac also breaks between CJK characters, where there is no space to restore. + $cjk_break = preg_match( '/' . self::BREAKABLE_CHARS . '$/u', $line ) && preg_match( '/^' . self::BREAKABLE_CHARS . '/u', $next_line ); + + if ( '-' !== substr( $line, -1 ) && ! $cjk_break ) { $line .= ' '; } - $line .= ltrim( array_shift( $lines ) ); + $line .= $next_line; } } diff --git a/common/includes/tests/slack/trac/comment-handler.php b/common/includes/tests/slack/trac/comment-handler.php index 57918a6e3f..964944bbb4 100644 --- a/common/includes/tests/slack/trac/comment-handler.php +++ b/common/includes/tests/slack/trac/comment-handler.php @@ -67,7 +67,7 @@ public function test_last_line_is_kept( array $body, string $comment, array $cha */ public function data_last_line_is_kept(): array { return array( - 'comment' => array( + 'comment' => array( array( '', 'Comment (by obenland):', @@ -79,7 +79,7 @@ public function data_last_line_is_kept(): array { "I started such a thread.\n\nSo it was closed.", array(), ), - 'changes only' => array( + 'changes only' => array( array( 'Changes (by obenland):', '', @@ -92,7 +92,7 @@ public function data_last_line_is_kept(): array { '_*status:*_ new => assigned', ), ), - 'wrapped attachment' => array( + 'wrapped attachment' => array( array( 'Changes (by adamsilverstein):', '', @@ -104,6 +104,18 @@ public function data_last_line_is_kept(): array { '_*attachment:*_ `Monosnap Site Health - Info ‹ My... Website — WordPress 2024-07-15 15-14-25.jpg` added', ), ), + 'wrapped CJK attachment' => array( + array( + 'Changes (by obenland):', + '', + ' * Attachment "ワードプレスのサイトヘルス情報画面のスクリーンショット二〇二', + ' 四年七月十五日撮影分.png" added.', + ), + '', + array( + '_*attachment:*_ `ワードプレスのサイトヘルス情報画面のスクリーンショット二〇二四年七月十五日撮影分.png` added', + ), + ), ); } }