From fded360d843d9d4ba16a4dee93ed8e798aa804cb Mon Sep 17 00:00:00 2001 From: Aleksander Machniak Date: Mon, 6 Jan 2020 20:33:35 +0100 Subject: [PATCH] Fix so messages in threads with no root aren't displayed separately (#4999) --- CHANGELOG | 1 + program/lib/Roundcube/rcube_result_thread.php | 31 ++++++++++++++--- tests/Framework/ResultThread.php | 34 +++++++++++++++++++ 3 files changed, 62 insertions(+), 4 deletions(-) diff --git a/CHANGELOG b/CHANGELOG index 7544c0162..350052bd0 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -9,6 +9,7 @@ CHANGELOG Roundcube Webmail - Templates: Make [space][slash] ending of condition objects optional (#6954) - Password: Make chpass-wrapper.py Python 3 compatible (#7135) - Elastic: Fix bug where it was possible to switch editor mode when 'htmleditor' was in 'dont_override' (#7143) +- Fix so messages in threads with no root aren't displayed separately (#4999) RELEASE 1.4.2 ------------- diff --git a/program/lib/Roundcube/rcube_result_thread.php b/program/lib/Roundcube/rcube_result_thread.php index 2ebd48ca8..11adda667 100644 --- a/program/lib/Roundcube/rcube_result_thread.php +++ b/program/lib/Roundcube/rcube_result_thread.php @@ -573,9 +573,9 @@ class rcube_result_thread $end = strlen($str); } - // Let's try to store data in max. compacted stracture as a string, + // Let's try to store data in max. compacted structure as a string, // arrays handling is much more expensive - // For the following structure: THREAD (2)(3 6 (4 23)(44 7 96)) + // For the following structure: THREAD (2)(3 6 (4 23)(44 7 96))((11)(12)) // -- 2 // -- 3 // \-- 6 @@ -585,8 +585,11 @@ class rcube_result_thread // \-- 44 // \-- 7 // \-- 96 + // -- 11 + // \-- 12 // - // The output will be: 2,3^1:6^2:4^3:23^2:44^3:7^4:96 + // The output will be: 2 3~1:6~2:4~3:23~2:44~3:7~4:96 11~1:12 + // Note: The "11" thread has no root, we use the first message as root if ($str[$begin] != '(') { // find next bracket @@ -622,6 +625,7 @@ class rcube_result_thread // @TODO: write error to the log or maybe set $this->raw_data = null; return $node; } + $p1 = strpos($str, '(', $off); if ($p1 !== false && $p1 < $p) { $off = $p1 + 1; @@ -633,7 +637,26 @@ class rcube_result_thread } } - $thread = $this->parse_thread($str, $start + 1, $off - 1, $depth); + // Handle threads with missing parent by using first message as root + if (substr_compare($str, '((', $start, 2) === 0) { + // Extract the current thread, e.g. "((1)(2))" + $thread = substr($str, $start, $off - $start); + // Length of the first token, e.g. "(1)" + $len = strspn($thread, '(0123456789', 1) + 1; + // Extract the token and modify it to look like a thread root + $token = substr($thread, 1, $len); + // Warning: The order is important + $token = str_replace('(', '', $token); + $token = str_replace(' ', ' (', $token); + $token = str_replace(')', ' ', $token); + $thread = substr_replace($thread, $token, 1, $len); + // Parse the thread + $thread = $this->parse_thread($thread, 0, 0, $depth); + } + else { + $thread = $this->parse_thread($str, $start + 1, $off - 1, $depth); + } + if ($thread) { if (!$depth) { if ($node) { diff --git a/tests/Framework/ResultThread.php b/tests/Framework/ResultThread.php index 5b6c7bafd..45c75ecc5 100644 --- a/tests/Framework/ResultThread.php +++ b/tests/Framework/ResultThread.php @@ -30,6 +30,7 @@ class Framework_ResultThread extends PHPUnit\Framework\TestCase $this->assertSame(false, $object->is_error(), "Object is error"); $this->assertSame(1721, $object->max(), "Max message UID"); $this->assertSame(1, $object->min(), "Min message UID"); + $this->assertSame(731, $object->count(), "Threads count"); $this->assertSame(1721, $object->count_messages(), "Messages count"); $this->assertSame(1691, $object->exists(1720, true), "Message exists"); $this->assertSame(true, $object->exists(1720), "Message exists (bool)"); @@ -37,6 +38,39 @@ class Framework_ResultThread extends PHPUnit\Framework\TestCase $this->assertSame(1719, $object->get_element('LAST'), "Get last element"); $this->assertSame(14, (int) $object->get_element(2), "Get specified element"); + $tree = $object->get_tree(); + $expected = [ + 4 => [ + 18 => [ + 39 => [ + 100 => [] + ] + ] + ], + 5 => [ + 6 => [], + 8 => [ + 11 => [], + 13 => [ + 15 => [] + ], + 465 => [] + ], + 209 => [] + ], + 19 => [ + 314 => [] + ] + ]; + + $this->assertSame([], $tree[1]); + $this->assertSame([], $tree[2]); + $this->assertSame([], $tree[14]); + $this->assertSame([], $tree[3]); + $this->assertSame($expected[4], $tree[4]); + $this->assertSame($expected[5], $tree[5]); + $this->assertSame($expected[19], $tree[19]); + $clone = clone $object; $clone->filter(array(7)); $clone = $clone->get_tree();