diff --git a/CHANGELOG.md b/CHANGELOG.md index deec23cb2..fe4463eb3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -35,6 +35,7 @@ - Bugfix: Fixed `/clearmessages` not working with more than one window. (#5489) - Bugfix: Fixed splits staying paused after unfocusing Chatterino in certain configurations. (#5504) - Bugfix: Links with invalid characters in the domain are no longer detected. (#5509) +- Bugfix: Fixed janky selection for messages with RTL segments (selection is still wrong, but consistently wrong). (#5525) - Dev: Update Windows build from Qt 6.5.0 to Qt 6.7.1. (#5420) - Dev: Update vcpkg build Qt from 6.5.0 to 6.7.0, boost from 1.83.0 to 1.85.0, openssl from 3.1.3 to 3.3.0. (#5422) - Dev: Unsingletonize `ISoundController`. (#5462) diff --git a/src/messages/layouts/MessageLayoutContainer.cpp b/src/messages/layouts/MessageLayoutContainer.cpp index 3c2bd122d..0f8aa498f 100644 --- a/src/messages/layouts/MessageLayoutContainer.cpp +++ b/src/messages/layouts/MessageLayoutContainer.cpp @@ -56,6 +56,8 @@ void MessageLayoutContainer::beginLayout(int width, float scale, this->currentWordId_ = 0; this->canAddMessages_ = true; this->isCollapsed_ = false; + this->lineContainsRTL_ = false; + this->anyReorderingDone_ = false; } void MessageLayoutContainer::endLayout() @@ -105,6 +107,17 @@ void MessageLayoutContainer::endLayout() { this->elements_.back()->setTrailingSpace(false); } + + if (this->anyReorderingDone_) + { + std::ranges::sort(this->elements_, [](const auto &a, const auto &b) { + if (a->getLine() == b->getLine()) + { + return a->getRect().x() < b->getRect().x(); + } + return a->getLine() < b->getLine(); + }); + } } void MessageLayoutContainer::addElement(MessageLayoutElement *element) @@ -137,6 +150,7 @@ void MessageLayoutContainer::breakLine() } } this->lineContainsRTL_ = false; + this->anyReorderingDone_ = true; } int xOffset = 0; diff --git a/src/messages/layouts/MessageLayoutContainer.hpp b/src/messages/layouts/MessageLayoutContainer.hpp index 7cbcaf9fb..7c14faf40 100644 --- a/src/messages/layouts/MessageLayoutContainer.hpp +++ b/src/messages/layouts/MessageLayoutContainer.hpp @@ -357,6 +357,9 @@ private: /// linebreak after which it's reset to `false`. bool lineContainsRTL_ = false; + /// True if there was any RTL/LTR reordering done in this container + bool anyReorderingDone_ = false; + /// @brief The direction of the text in this container. /// /// This starts off as neutral until an element is encountered that is diff --git a/tests/src/MessageLayoutContainer.cpp b/tests/src/MessageLayoutContainer.cpp index b72934c76..b0aa4f30e 100644 --- a/tests/src/MessageLayoutContainer.cpp +++ b/tests/src/MessageLayoutContainer.cpp @@ -118,34 +118,31 @@ TEST_P(MessageLayoutContainerTest, RtlReordering) MessageElementFlag::TwitchEmote, }); } - container.breakLine(); + container.endLayout(); ASSERT_EQ(container.line_, 1) << "unexpected linebreak"; - // message layout elements ordered by x position - std::vector ordered; - ordered.reserve(container.elements_.size()); + int x = -1; for (const auto &el : container.elements_) { - ordered.push_back(el.get()); + ASSERT_LT(x, el->getRect().x()); + x = el->getRect().x(); } - std::ranges::sort(ordered, [](auto *a, auto *b) { - return a->getRect().x() < b->getRect().x(); - }); - QString got; - for (const auto &el : ordered) + for (const auto &el : container.elements_) { if (!got.isNull()) { got.append(' '); } - if (dynamic_cast(el)) + if (dynamic_cast(el.get())) { el->addCopyTextToString(got); - ASSERT_TRUE(got.endsWith(' ')); - got.chop(1); + if (el->hasTrailingSpace()) + { + got.chop(1); + } } else {