diff --git a/CHANGELOG.md b/CHANGELOG.md index 19dcbe7a7..7cac0b0f8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -83,6 +83,7 @@ - Bugfix: Fixed badge highlight changes not immediately being reflected. (#5110) - Bugfix: Fixed emotes being reloaded when pressing "Cancel" in the settings dialog, causing a slowdown. (#5240) - Bugfix: Fixed some emotes not appearing when using _Ignores_. (#4965, #5126) +- Bugfix: Fixed double-click selection not selecting words that were split onto multiple lines correctly. (#5243) - Bugfix: Fixed lookahead/-behind not working in _Ignores_. (#4965, #5126) - Bugfix: Fixed Image Uploader accidentally deleting images with some hosts when link resolver was enabled. (#4971) - Bugfix: Fixed rare crash with Image Uploader when closing a split right after starting an upload. (#4971) diff --git a/src/messages/MessageElement.cpp b/src/messages/MessageElement.cpp index e895f7630..56d1e2ed3 100644 --- a/src/messages/MessageElement.cpp +++ b/src/messages/MessageElement.cpp @@ -454,7 +454,7 @@ TextElement::TextElement(const QString &text, MessageElementFlags flags, void TextElement::addToContainer(MessageLayoutContainer &container, MessageElementFlags flags) { - auto *app = getApp(); + auto *app = getIApp(); if (flags.hasAny(this->getFlags())) { @@ -463,6 +463,8 @@ void TextElement::addToContainer(MessageLayoutContainer &container, for (const auto &word : this->words_) { + auto wordId = container.nextWordId(); + auto getTextLayoutElement = [&](QString text, int width, bool hasTrailingSpace) { auto color = this->color_.getColor(*app->getThemes()); @@ -473,6 +475,7 @@ void TextElement::addToContainer(MessageLayoutContainer &container, this->style_, container.getScale()); e->setTrailingSpace(hasTrailingSpace); e->setText(text); + e->setWordId(wordId); return e; }; diff --git a/src/messages/layouts/MessageLayout.cpp b/src/messages/layouts/MessageLayout.cpp index efc1d1b56..126bb40c9 100644 --- a/src/messages/layouts/MessageLayout.cpp +++ b/src/messages/layouts/MessageLayout.cpp @@ -443,12 +443,31 @@ void MessageLayout::deleteCache() // returns nullptr if none was found // fourtf: this should return a MessageLayoutItem -const MessageLayoutElement *MessageLayout::getElementAt(QPoint point) +const MessageLayoutElement *MessageLayout::getElementAt(QPoint point) const { // go through all words and return the first one that contains the point. return this->container_.getElementAt(point); } +std::pair MessageLayout::getWordBounds( + const MessageLayoutElement *hoveredElement, QPoint relativePos) const +{ + // An element with wordId != -1 can be multiline, so we need to check all + // elements in the container + if (hoveredElement->getWordId() != -1) + { + return this->container_.getWordBounds(hoveredElement); + } + + const auto wordStart = this->getSelectionIndex(relativePos) - + hoveredElement->getMouseOverIndex(relativePos); + const auto selectionLength = hoveredElement->getSelectionIndexCount(); + const auto length = hoveredElement->hasTrailingSpace() ? selectionLength - 1 + : selectionLength; + + return {wordStart, wordStart + length}; +} + size_t MessageLayout::getLastCharacterIndex() const { return this->container_.getLastCharacterIndex(); diff --git a/src/messages/layouts/MessageLayout.hpp b/src/messages/layouts/MessageLayout.hpp index 8a177227f..f54f57d2a 100644 --- a/src/messages/layouts/MessageLayout.hpp +++ b/src/messages/layouts/MessageLayout.hpp @@ -70,7 +70,21 @@ public: * * If no element is found at the given point, this returns a null pointer */ - const MessageLayoutElement *getElementAt(QPoint point); + const MessageLayoutElement *getElementAt(QPoint point) const; + + /** + * @brief Returns the word bounds of the given element + * + * The first value is the index of the first character in the word, + * the second value is the index of the character after the last character in the word. + * + * Given the word "abc" by itself, we would return (0, 3) + * + * V V + * "abc " + */ + std::pair getWordBounds( + const MessageLayoutElement *hoveredElement, QPoint relativePos) const; /** * Get the index of the last character in this message's container diff --git a/src/messages/layouts/MessageLayoutContainer.cpp b/src/messages/layouts/MessageLayoutContainer.cpp index 17b9b795d..29d70e0a1 100644 --- a/src/messages/layouts/MessageLayoutContainer.cpp +++ b/src/messages/layouts/MessageLayoutContainer.cpp @@ -51,6 +51,7 @@ void MessageLayoutContainer::beginLayout(int width, float scale, this->textLineHeight_ = mediumFontMetrics.height(); this->spaceWidth_ = mediumFontMetrics.horizontalAdvance(' '); this->dotdotdotWidth_ = mediumFontMetrics.horizontalAdvance("..."); + this->currentWordId_ = 0; this->canAddMessages_ = true; this->isCollapsed_ = false; this->wasPrevReversed_ = false; @@ -456,6 +457,50 @@ size_t MessageLayoutContainer::getFirstMessageCharacterIndex() const return index; } +std::pair MessageLayoutContainer::getWordBounds( + const MessageLayoutElement *hoveredElement) const +{ + if (this->elements_.empty()) + { + return {0, 0}; + } + + size_t index = 0; + size_t wordStart = 0; + + for (; index < this->elements_.size(); index++) + { + const auto &element = this->elements_[index]; + if (element->getWordId() == hoveredElement->getWordId()) + { + break; + } + + wordStart += element->getSelectionIndexCount(); + } + + size_t wordEnd = wordStart; + + for (; index < this->elements_.size(); index++) + { + const auto &element = this->elements_[index]; + if (element->getWordId() != hoveredElement->getWordId()) + { + break; + } + + wordEnd += element->getSelectionIndexCount(); + } + + const auto *lastElementInSelection = this->elements_[index - 1].get(); + if (lastElementInSelection->hasTrailingSpace()) + { + wordEnd--; + } + + return {wordStart, wordEnd}; +} + size_t MessageLayoutContainer::getLastCharacterIndex() const { if (this->lines_.empty()) @@ -505,6 +550,11 @@ int MessageLayoutContainer::remainingWidth() const this->currentX_; } +int MessageLayoutContainer::nextWordId() +{ + return this->currentWordId_++; +} + void MessageLayoutContainer::addElement(MessageLayoutElement *element, const bool forceAdd, const int prevIndex) diff --git a/src/messages/layouts/MessageLayoutContainer.hpp b/src/messages/layouts/MessageLayoutContainer.hpp index be765da85..ed3c1a7a6 100644 --- a/src/messages/layouts/MessageLayoutContainer.hpp +++ b/src/messages/layouts/MessageLayoutContainer.hpp @@ -111,6 +111,20 @@ struct MessageLayoutContainer { */ size_t getFirstMessageCharacterIndex() const; + /** + * @brief Returns the word bounds of the given element + * + * The first value is the index of the first character in the word, + * the second value is the index of the character after the last character in the word. + * + * Given the word "abc" by itself, we would return (0, 3) + * + * V V + * "abc " + */ + std::pair getWordBounds( + const MessageLayoutElement *hoveredElement) const; + /** * Get the index of the last character in this message * This is the sum of all the characters in `elements_` @@ -154,6 +168,11 @@ struct MessageLayoutContainer { */ int remainingWidth() const; + /** + * Returns the id of the next word that can be added to this container + */ + int nextWordId(); + private: struct Line { /** @@ -272,6 +291,7 @@ private: int spaceWidth_ = 4; int textLineHeight_ = 0; int dotdotdotWidth_ = 0; + int currentWordId_ = 0; bool canAddMessages_ = true; bool isCollapsed_ = false; bool wasPrevReversed_ = false; diff --git a/src/messages/layouts/MessageLayoutElement.cpp b/src/messages/layouts/MessageLayoutElement.cpp index ffa949d7f..31b7d4fe5 100644 --- a/src/messages/layouts/MessageLayoutElement.cpp +++ b/src/messages/layouts/MessageLayoutElement.cpp @@ -108,6 +108,16 @@ FlagsEnum MessageLayoutElement::getFlags() const return this->creator_.getFlags(); } +int MessageLayoutElement::getWordId() const +{ + return this->wordId_; +} + +void MessageLayoutElement::setWordId(int wordId) +{ + this->wordId_ = wordId; +} + // // IMAGE // diff --git a/src/messages/layouts/MessageLayoutElement.hpp b/src/messages/layouts/MessageLayoutElement.hpp index bbb45302f..de68a43f7 100644 --- a/src/messages/layouts/MessageLayoutElement.hpp +++ b/src/messages/layouts/MessageLayoutElement.hpp @@ -71,6 +71,9 @@ public: const QString &getText() const; FlagsEnum getFlags() const; + int getWordId() const; + void setWordId(int wordId); + protected: bool trailingSpace = true; @@ -83,6 +86,13 @@ private: * The line of the container this element is laid out at */ size_t line_{}; + + /// @brief ID of a word inside its container + /// + /// One word has exactly one ID that is used to identify elements created + /// from the same word (due to wrapping). + /// IDs are unique in a MessageLayoutContainer. + int wordId_ = -1; }; // IMAGE diff --git a/src/widgets/helper/ChannelView.cpp b/src/widgets/helper/ChannelView.cpp index 3a38cadf0..3cf64e116 100644 --- a/src/widgets/helper/ChannelView.cpp +++ b/src/widgets/helper/ChannelView.cpp @@ -290,23 +290,6 @@ qreal highlightEasingFunction(qreal progress) return 1.0 + pow((20.0 / 9.0) * (0.5 * progress - 0.5), 3.0); } -/// @return the start and end of the word bounds -std::pair getWordBounds(MessageLayout *layout, - const MessageLayoutElement *element, - const QPoint &relativePos) -{ - assert(layout != nullptr); - assert(element != nullptr); - - const auto wordStart = layout->getSelectionIndex(relativePos) - - element->getMouseOverIndex(relativePos); - const auto selectionLength = element->getSelectionIndexCount(); - const auto length = - element->hasTrailingSpace() ? selectionLength - 1 : selectionLength; - - return {wordStart, wordStart + length}; -} - } // namespace namespace chatterino { @@ -1827,7 +1810,7 @@ void ChannelView::mouseMoveEvent(QMouseEvent *event) if (this->isDoubleClick_ && hoverLayoutElement) { auto [wordStart, wordEnd] = - getWordBounds(layout.get(), hoverLayoutElement, relativePos); + layout->getWordBounds(hoverLayoutElement, relativePos); auto hoveredWord = Selection{SelectionItem(messageIndex, wordStart), SelectionItem(messageIndex, wordEnd)}; // combined selection spanning from initially selected word to hoveredWord @@ -2657,7 +2640,8 @@ void ChannelView::mouseDoubleClickEvent(QMouseEvent *event) } auto [wordStart, wordEnd] = - getWordBounds(layout.get(), hoverLayoutElement, relativePos); + layout->getWordBounds(hoverLayoutElement, relativePos); + this->doubleClickSelection_ = {SelectionItem(messageIndex, wordStart), SelectionItem(messageIndex, wordEnd)}; this->setSelection(this->doubleClickSelection_); diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index 53ffd5e1a..f1f80cf0e 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -39,6 +39,7 @@ set(test_SOURCES ${CMAKE_CURRENT_LIST_DIR}/src/NotebookTab.cpp ${CMAKE_CURRENT_LIST_DIR}/src/SplitInput.cpp ${CMAKE_CURRENT_LIST_DIR}/src/LinkInfo.cpp + ${CMAKE_CURRENT_LIST_DIR}/src/MessageLayout.cpp # Add your new file above this line! ) diff --git a/tests/src/MessageLayout.cpp b/tests/src/MessageLayout.cpp new file mode 100644 index 000000000..9ce0c7f21 --- /dev/null +++ b/tests/src/MessageLayout.cpp @@ -0,0 +1,90 @@ +#include "messages/layouts/MessageLayout.hpp" + +#include "Application.hpp" +#include "controllers/accounts/AccountController.hpp" +#include "messages/MessageBuilder.hpp" +#include "messages/MessageElement.hpp" +#include "mocks/EmptyApplication.hpp" +#include "singletons/Emotes.hpp" +#include "singletons/Fonts.hpp" +#include "singletons/Settings.hpp" +#include "singletons/Theme.hpp" +#include "singletons/WindowManager.hpp" + +#include +#include +#include + +#include + +using namespace chatterino; + +namespace { + +class MockApplication : mock::EmptyApplication +{ +public: + MockApplication() + : settings(this->settingsDir.filePath("settings.json")) + , fonts(this->settings) + , windowManager(this->paths_) + { + } + Theme *getThemes() override + { + return &this->theme; + } + + Fonts *getFonts() override + { + return &this->fonts; + } + + WindowManager *getWindows() override + { + return &this->windowManager; + } + + Settings settings; + Theme theme; + Fonts fonts; + WindowManager windowManager; +}; + +constexpr int WIDTH = 300; + +class MessageLayoutTest +{ +public: + // "aaaaaaaa bbbbbbbb cccccccc" + MessageLayoutTest(const QString &text) + { + MessageBuilder builder; + builder.append( + std::make_unique(text, MessageElementFlag::Text)); + this->layout = std::make_unique(builder.release()); + this->layout->layout(WIDTH, 1, MessageElementFlag::Text, false); + } + + MockApplication mockApplication; + std::unique_ptr layout; +}; + +} // namespace + +TEST(TextElement, BasicCase) +{ + auto test = MessageLayoutTest("abc"); + + // Simulate we are clicking on the first word + auto point = QPoint(WIDTH / 20, test.layout->getHeight() / 2); + + const auto *hoveredElement = test.layout->getElementAt(point); + ASSERT_NE(hoveredElement, nullptr); + + const auto [wordStart, wordEnd] = + test.layout->getWordBounds(hoveredElement, point); + + EXPECT_EQ(wordStart, 0); + EXPECT_EQ(wordEnd, 3); +}