Skip to content

Commit

Permalink
Merge PR #6289: FIX(client, ui): fix text selection in chat log
Browse files Browse the repository at this point in the history
In v1.4, the logic surrounding when entries in the chat log are put in
their own text frame was changed, introducing a bug where elements such
as lists would not be terminated correctly (#4491).

The fix for this issue was to put every chat log entry into its own text
frame, but Qt inserts a blank line between adjacent frames resulting in
too much space between subsequent chat entries.

There is no convenient Qt API (as far as I know) to disable this
behaviour, so my second attempt to resolve this (PR #5820) set the font
size to near zero for the blank lines between entries.

This worked on Linux, but broke on (at least) Windows (#5886).

PR #5927 fixed the issue by calling `setVisible(false)` on the blank
lines instead of changing the font size, but this introduced a regression
to text selection in the chat log (#6045).

This PR deals with the empty lines between entries by setting their
*line height* to zero using a call to
`setLineHeight(0, QTextBlockFormat::FixedHeight)`.

https://doc.qt.io/qt-5/qtextblockformat.html#LineHeightTypes-enum

According to the Qt documentation, this method allows setting a fixed
line height in pixels. Based on my testing on Linux, this solves the
problem without introducing any new regressions.

This should still be tested on Windows before v1.5, just in case.

Fixes #6045
  • Loading branch information
Krzmbrzl authored Jan 2, 2024
2 parents bc03c56 + 0c224e8 commit 0d1529e
Showing 1 changed file with 20 additions and 25 deletions.
45 changes: 20 additions & 25 deletions src/mumble/Log.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -406,10 +406,6 @@ Log::Log(QObject *p) : QObject(p) {
#endif
uiLastId = 0;
qdDate = QDate::currentDate();

// remove gap above first chat message; the gaps below
// each chat message are handled within `Log::log`.
Global::get().mw->qteLog->document()->firstBlock().setVisible(false);
}

// Display order in settingsscreen, allows to insert new events without breaking config-compatibility with older
Expand Down Expand Up @@ -728,12 +724,29 @@ void Log::log(MsgType mt, const QString &console, const QString &terse, bool own
const int oldscrollvalue = tlog->getLogScroll();
const bool scroll = (oldscrollvalue == tlog->getLogScrollMaximum());

// A newline is inserted after each frame, but this spaces out the
// log entries too much, so the line height is set to zero to reduce
// the space between log entries. This line height is only set for the
// blank lines between entries, not for entries themselves.
//
// NOTE: All further log entries must go in a new text frame.
// Otherwise, they will not display correctly as a result of having
// line height equal to 0 for the current block.
QTextBlockFormat bf = tc.blockFormat();
bf.setLineHeight(0, QTextBlockFormat::FixedHeight);
bf.setTopMargin(0);
bf.setBottomMargin(0);

// Set the line height of the leading blank line to zero
tc.setBlockFormat(bf);

if (qdDate != dt.date()) {
qdDate = dt.date();
tc.insertBlock();
tc.insertFrame(qttf);
tc.insertHtml(
tr("[Date changed to %1]\n").arg(QLocale().toString(qdDate, QLocale::ShortFormat).toHtmlEscaped()));
tc.movePosition(QTextCursor::End);
tc.setBlockFormat(bf);
}

// Convert CRLF to unix-style LF and old mac-style LF (single \r) to unix-style as well
Expand All @@ -760,26 +773,8 @@ void Log::log(MsgType mt, const QString &console, const QString &terse, bool own
tc.movePosition(QTextCursor::End);
Global::get().mw->qteLog->setTextCursor(tc);

// Qt's document model for [Rich Text Documents][RT] is based on blocks and frames.
// You always have a root frame and at least one block per frame:
//
// [Document]
// +--> [Root frame]
// +--> [Block]
// +--> [Frame]
// +--> [Block]
// +--> [Frame]
// +--> [Block]
//
// [RT]: https://doc.qt.io/qt-5/richtext-structure.html
//
// However, the issue is that the blocks between the frames are mandatory. They lead
// to additional gaps, especially on Windows, where `QTextCursor::setBlockCharFormat`
// cannot be used to decrease the trailing block's size.
//
// Fortunately, the `tlog`/`LogTextBrowser` is a `QTextBrowser` with a `QDocument*`.
// This allows us to hide the last block created by `QTextCursor::insertFrame()`.
tlog->document()->lastBlock().setVisible(false);
// Set the line height of the trailing blank line to zero
tc.setBlockFormat(bf);

if (scroll || ownMessage)
tlog->scrollLogToBottom();
Expand Down

0 comments on commit 0d1529e

Please sign in to comment.