Fix pre-release issues with attachment viewer (#12244)

* Fix translation issues for "FIT" and "New Attachment" in attachment editor

* Fix markdown preview persistence and enable external links in attachment editor

* Update preview panel if manually moved from collapsed position

* Match edit view scroll position (by percentage) when changed. This ensures the preview remains in relative sync with the edited document, for example when a large amount of HTML reduces down to a short preview document.

* Fix default preview size to be half the width of the edit widget.

* Set tab stop to 10 and remove base ui file

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: Jonathan White <support@dmapps.us>

share/translations/keepassxc_en.ts

@@ -4110,6 +4110,10 @@ Would you like to overwrite the existing attachment?</source>
         <source>Save…</source>
         <translation type="unfinished"></translation>
     </message>
+    <message>
+        <source>New Attachment</source>
+        <translation type="unfinished"></translation>
+    </message>
 </context>
 <context>
     <name>EntryAttributesModel</name>
@@ -4679,6 +4683,10 @@ You can enable the DuckDuckGo website icon service in the security section of th
         <source>Zoom:</source>
         <translation type="unfinished"></translation>
     </message>
+    <message>
+        <source>Fit</source>
+        <translation type="unfinished"></translation>
+    </message>
 </context>
 <context>
     <name>ImportWizard</name>
@@ -9261,10 +9269,6 @@ This option is deprecated, use --set-key-file instead.</source>
         <comment>TOTP</comment>
         <translation type="unfinished"></translation>
     </message>
-    <message>
-        <source>Fit</source>
-        <translation type="unfinished"></translation>
-    </message>
     <message numerus="yes">
         <source>%1 character(s)</source>
         <translation type="unfinished">

src/gui/entry/EntryAttachmentsWidget.cpp

@@ -37,7 +37,6 @@
 
 namespace
 {
-    constexpr const char* DefaultName = "New Attachment";
     constexpr const char* Suffix = ".txt";
 
     QString generateUniqueName(const QString& name, const QStringList& existingNames)
@@ -210,7 +209,7 @@ void EntryAttachmentsWidget::newAttachments()
     }
 
     // Create a temporary file to allow the user to edit the attachment
-    auto newFileName = generateUniqueName(DefaultName, m_entryAttachments->keys());
+    auto newFileName = generateUniqueName(tr("New Attachment"), m_entryAttachments->keys());
     m_entryAttachments->set(newFileName, QByteArray());
 
     auto currentIndex = m_attachmentsModel->index(m_attachmentsModel->rowByKey(newFileName), 0);

src/gui/entry/attachments/ImageAttachmentsWidget.cpp

@@ -37,8 +37,6 @@ namespace
     constexpr std::array ZoomList = {0.25, 0.5, 0.75, 1.0, 2.0};
     constexpr double WheelZoomStep = 1.1;
 
-    const QString FitText = QObject::tr("Fit");
-
     QString formatZoomText(double zoomFactor)
     {
         return QString("%1%").arg(QString::number(zoomFactor * 100, 'f', 0));
@@ -92,9 +90,10 @@ void ImageAttachmentsWidget::initZoomComboBox()
 {
     m_ui->zoomComboBox->clear();
 
-    auto textWidth = m_ui->zoomComboBox->fontMetrics().horizontalAdvance(FitText);
+    auto fitText = tr("Fit");
+    auto textWidth = m_ui->zoomComboBox->fontMetrics().horizontalAdvance(fitText);
 
-    m_ui->zoomComboBox->addItem(FitText, 0.0);
+    m_ui->zoomComboBox->addItem(fitText, 0.0);
 
     for (const auto& zoom : ZoomList) {
         auto zoomText = formatZoomText(zoom);
@@ -146,7 +145,7 @@ void ImageAttachmentsWidget::onZoomChanged(const QString& zoomText)
 {
     auto zoomFactor = 1.0;
 
-    if (zoomText == FitText) {
+    if (zoomText == tr("Fit")) {
         m_ui->imagesView->enableAutoFitInView();
 
         zoomFactor = std::min(m_ui->imagesView->calculateFitInViewFactor(), zoomFactor);

src/gui/entry/attachments/TextAttachmentsEditWidget.cpp

@@ -19,8 +19,8 @@
 #include "ui_TextAttachmentsEditWidget.h"
 
 #include <QPushButton>
+#include <QScrollBar>
 #include <QTextEdit>
-#include <qwidget.h>
 
 TextAttachmentsEditWidget::TextAttachmentsEditWidget(QWidget* parent)
     : QWidget(parent)
@@ -30,6 +30,11 @@ TextAttachmentsEditWidget::TextAttachmentsEditWidget(QWidget* parent)
 
     connect(m_ui->attachmentsTextEdit, &QTextEdit::textChanged, this, &TextAttachmentsEditWidget::textChanged);
     connect(m_ui->previewPushButton, &QPushButton::clicked, this, &TextAttachmentsEditWidget::previewButtonClicked);
+    connect(m_ui->attachmentsTextEdit->verticalScrollBar(), &QScrollBar::valueChanged, this, [this](int value) {
+        // Return a percentage of document scroll
+        auto percent = static_cast<double>(value) / m_ui->attachmentsTextEdit->verticalScrollBar()->maximum();
+        emit scrollChanged(percent);
+    });
 }
 
 TextAttachmentsEditWidget::~TextAttachmentsEditWidget() = default;

src/gui/entry/attachments/TextAttachmentsEditWidget.h

@@ -39,6 +39,7 @@ public:
 
 signals:
     void textChanged();
+    void scrollChanged(double percent);
     void previewButtonClicked(bool isChecked);
 
 private:

src/gui/entry/attachments/TextAttachmentsEditWidget.ui

@@ -57,7 +57,11 @@
     </layout>
    </item>
    <item>
-    <widget class="QTextEdit" name="attachmentsTextEdit"/>
+    <widget class="QTextEdit" name="attachmentsTextEdit">
+     <property name="tabStopDistance">
+      <double>10.000000000000000</double>
+     </property>
+    </widget>
    </item>
   </layout>
  </widget>

src/gui/entry/attachments/TextAttachmentsPreviewWidget.cpp

@@ -23,8 +23,10 @@
 #include <QComboBox>
 #include <QDebug>
 #include <QMetaEnum>
+#include <QScrollBar>
 #include <QSortFilterProxyModel>
 #include <QStandardItemModel>
+#include <QTimer>
 
 namespace
 {
@@ -48,6 +50,7 @@ namespace
 TextAttachmentsPreviewWidget::TextAttachmentsPreviewWidget(QWidget* parent)
     : QWidget(parent)
     , m_ui(new Ui::TextAttachmentsPreviewWidget())
+    , m_userManuallySelectedType(false)
 {
     m_ui->setupUi(this);
 
@@ -88,10 +91,13 @@ void TextAttachmentsPreviewWidget::initTypeCombobox()
     filterProxyMode->sort(0, Qt::SortOrder::DescendingOrder);
     m_ui->typeComboBox->setModel(filterProxyMode);
 
-    connect(m_ui->typeComboBox,
-            QOverload<int>::of(&QComboBox::currentIndexChanged),
-            this,
-            &TextAttachmentsPreviewWidget::onTypeChanged);
+    connect(m_ui->typeComboBox, QOverload<int>::of(&QComboBox::currentIndexChanged), this, [this](int index) {
+        m_userManuallySelectedType = true;
+        onTypeChanged(index);
+    });
+
+    // Configure text browser to open external links
+    m_ui->previewTextBrowser->setOpenExternalLinks(true);
 
     m_ui->typeComboBox->setCurrentIndex(m_ui->typeComboBox->findData(PlainText));
 
@@ -100,11 +106,17 @@ void TextAttachmentsPreviewWidget::initTypeCombobox()
 
 void TextAttachmentsPreviewWidget::updateUi()
 {
-    if (!m_attachment.name.isEmpty()) {
+    // Only auto-select format based on file extension if user hasn't manually chosen one
+    if (!m_userManuallySelectedType && !m_attachment.name.isEmpty()) {
         const auto mimeType = Tools::getMimeType(QFileInfo(m_attachment.name));
 
         auto index = m_ui->typeComboBox->findData(ConvertToPreviewTextType(mimeType));
+        if (index >= 0) {
+            // Temporarily block signals to avoid triggering manual selection flag
+            m_ui->typeComboBox->blockSignals(true);
             m_ui->typeComboBox->setCurrentIndex(index);
+            m_ui->typeComboBox->blockSignals(false);
+        }
     }
 
     onTypeChanged(m_ui->typeComboBox->currentIndex());
@@ -116,6 +128,7 @@ void TextAttachmentsPreviewWidget::onTypeChanged(int index)
         qWarning() << "TextAttachmentsPreviewWidget: Unknown text format";
     }
 
+    const auto scrollPos = m_ui->previewTextBrowser->verticalScrollBar()->value();
     const auto fileType = m_ui->typeComboBox->itemData(index).toInt();
     if (fileType == TextAttachmentsPreviewWidget::PreviewTextType::PlainText) {
         m_ui->previewTextBrowser->setPlainText(m_attachment.data);
@@ -130,4 +143,16 @@ void TextAttachmentsPreviewWidget::onTypeChanged(int index)
         m_ui->previewTextBrowser->setMarkdown(m_attachment.data);
     }
 #endif
+
+    // Delay setting the scrollbar position to ensure the text is rendered first
+    QTimer::singleShot(
+        100, this, [this, scrollPos] { m_ui->previewTextBrowser->verticalScrollBar()->setValue(scrollPos); });
+}
+
+void TextAttachmentsPreviewWidget::matchScroll(double percent)
+{
+    // Match the scroll position of the text browser to the given percentage
+    int maxScroll = m_ui->previewTextBrowser->verticalScrollBar()->maximum();
+    int newScrollPos = static_cast<int>(maxScroll * percent);
+    m_ui->previewTextBrowser->verticalScrollBar()->setValue(newScrollPos);
 }

src/gui/entry/attachments/TextAttachmentsPreviewWidget.h

@@ -48,6 +48,9 @@ public:
 
     Q_ENUM(PreviewTextType)
 
+public slots:
+    void matchScroll(double percent);
+
 private slots:
     void onTypeChanged(int index);
 
@@ -58,4 +61,5 @@ private:
     QScopedPointer<Ui::TextAttachmentsPreviewWidget> m_ui;
 
     attachments::Attachment m_attachment;
+    bool m_userManuallySelectedType;
 };

src/gui/entry/attachments/TextAttachmentsPreviewWidget.ui

@@ -61,7 +61,11 @@
     </layout>
    </item>
    <item>
-    <widget class="QTextBrowser" name="previewTextBrowser"/>
+    <widget class="QTextBrowser" name="previewTextBrowser">
+     <property name="tabStopDistance">
+      <double>10.000000000000000</double>
+     </property>
+    </widget>
    </item>
   </layout>
  </widget>

src/gui/entry/attachments/TextAttachmentsWidget.cpp

@@ -19,19 +19,16 @@
 #include "TextAttachmentsEditWidget.h"
 #include "TextAttachmentsPreviewWidget.h"
 
-#include "ui_TextAttachmentsWidget.h"
-
 #include <QSplitter>
 #include <QTextEdit>
 #include <QTimer>
+#include <QVBoxLayout>
 
 TextAttachmentsWidget::TextAttachmentsWidget(QWidget* parent)
     : QWidget(parent)
-    , m_ui(new Ui::TextAttachmentsWidget())
     , m_previewUpdateTimer(new QTimer(this))
     , m_mode(attachments::OpenMode::ReadOnly)
 {
-    m_ui->setupUi(this);
     initWidget();
 }
 
@@ -57,15 +54,25 @@ attachments::Attachment TextAttachmentsWidget::getAttachment() const
 void TextAttachmentsWidget::updateWidget()
 {
     if (m_mode == attachments::OpenMode::ReadOnly) {
+        // Only show the preview widget in read-only mode
         m_splitter->setSizes({0, 1});
         m_editWidget->hide();
+        m_previewWidget->openAttachment(m_attachment, m_mode);
     } else {
+        // Show the edit widget and hide the preview by default in read-write mode
         m_splitter->setSizes({1, 0});
         m_editWidget->show();
-    }
-
         m_editWidget->openAttachment(m_attachment, m_mode);
+    }
+}
+
+void TextAttachmentsWidget::updatePreviewWidget()
+{
+    m_previewVisible = isPreviewVisible();
+    if (m_previewVisible) {
+        m_attachment = m_editWidget->getAttachment();
         m_previewWidget->openAttachment(m_attachment, attachments::OpenMode::ReadOnly);
+    }
 }
 
 void TextAttachmentsWidget::initWidget()
@@ -78,20 +85,31 @@ void TextAttachmentsWidget::initWidget()
     m_previewUpdateTimer->setInterval(500);
 
     // Only update the preview after a set timeout and if it is visible
-    connect(m_previewUpdateTimer, &QTimer::timeout, this, [this] {
-        if (m_previewWidget->width() > 0) {
-            m_attachment = m_editWidget->getAttachment();
-            m_previewWidget->openAttachment(m_attachment, attachments::OpenMode::ReadOnly);
-        }
-    });
+    connect(m_previewUpdateTimer, &QTimer::timeout, this, &TextAttachmentsWidget::updatePreviewWidget);
+    connect(m_editWidget,
+            &TextAttachmentsEditWidget::scrollChanged,
+            m_previewWidget,
+            &TextAttachmentsPreviewWidget::matchScroll);
 
     connect(
         m_editWidget, &TextAttachmentsEditWidget::textChanged, m_previewUpdateTimer, QOverload<>::of(&QTimer::start));
 
     connect(m_editWidget, &TextAttachmentsEditWidget::previewButtonClicked, [this] {
-        const auto sizes = m_splitter->sizes();
-        const auto previewSize = sizes.value(1, 0) > 0 ? 0 : 1;
-        m_splitter->setSizes({1, previewSize});
+        // Split the display in half if showing the preview widget
+        const auto previewSize = isPreviewVisible() ? 0 : m_splitter->width() / 2;
+        const auto editSize = m_splitter->width() - previewSize;
+        m_splitter->setSizes({editSize, previewSize});
+        updatePreviewWidget();
+    });
+
+    // Check if the preview panel is manually collapsed or shown
+    connect(m_splitter, &QSplitter::splitterMoved, this, [this](int, int) {
+        // Trigger a preview update if it has become visible
+        auto visible = isPreviewVisible();
+        if (visible && !m_previewVisible) {
+            updatePreviewWidget();
+        }
+        m_previewVisible = visible;
     });
 
     m_splitter->addWidget(m_editWidget);
@@ -99,7 +117,17 @@ void TextAttachmentsWidget::initWidget()
     // Prevent collapsing of the edit widget
     m_splitter->setCollapsible(0, false);
 
-    m_ui->verticalLayout->addWidget(m_splitter);
+    // Setup this widget with the splitter
+    auto layout = new QVBoxLayout(this);
+    layout->setContentsMargins(0, 0, 0, 0);
+    layout->addWidget(m_splitter);
+    setLayout(layout);
+    setObjectName("TextAttachmentsWidget");
 
     updateWidget();
 }
+
+bool TextAttachmentsWidget::isPreviewVisible() const
+{
+    return m_previewWidget->width() > 0;
+}

src/gui/entry/attachments/TextAttachmentsWidget.h

@@ -43,15 +43,19 @@ public:
     void openAttachment(attachments::Attachment attachment, attachments::OpenMode mode);
     attachments::Attachment getAttachment() const;
 
-private:
-    void updateWidget();
-    void initWidget();
+private slots:
+    void updatePreviewWidget();
+
+private:
+    void initWidget();
+    void updateWidget();
+    bool isPreviewVisible() const;
 
-    QScopedPointer<Ui::TextAttachmentsWidget> m_ui;
     QPointer<QSplitter> m_splitter;
     QPointer<TextAttachmentsEditWidget> m_editWidget;
     QPointer<TextAttachmentsPreviewWidget> m_previewWidget;
     QPointer<QTimer> m_previewUpdateTimer;
+    bool m_previewVisible = false;
 
     attachments::Attachment m_attachment;
     attachments::OpenMode m_mode;

src/gui/entry/attachments/TextAttachmentsWidget.ui

@@ -1,36 +0,0 @@
-<?xml version="1.0" encoding="UTF-8"?>
-<ui version="4.0">
- <class>TextAttachmentsWidget</class>
- <widget class="QWidget" name="TextAttachmentsWidget">
-  <property name="geometry">
-   <rect>
-    <x>0</x>
-    <y>0</y>
-    <width>732</width>
-    <height>432</height>
-   </rect>
-  </property>
-  <property name="windowTitle">
-   <string/>
-  </property>
-  <layout class="QVBoxLayout" name="verticalLayout">
-   <property name="spacing">
-    <number>0</number>
-   </property>
-   <property name="leftMargin">
-    <number>0</number>
-   </property>
-   <property name="topMargin">
-    <number>0</number>
-   </property>
-   <property name="rightMargin">
-    <number>0</number>
-   </property>
-   <property name="bottomMargin">
-    <number>0</number>
-   </property>
-  </layout>
- </widget>
- <resources/>
- <connections/>
-</ui>

tests/gui/attachments/TestTextAttachmentsWidget.cpp

@@ -50,10 +50,18 @@ void TestTextAttachmentsWidget::testTextReadWriteWidget()
     QCOMPARE(attachments.name, Test.name);
     QCOMPARE(attachments.data, Test.data);
 
+    // Find preview widget and verify that it is in collapsed state
     auto previewWidget = qobject_cast<TextAttachmentsPreviewWidget*>(splitter->widget(1));
     QVERIFY(previewWidget);
-    attachments = previewWidget->getAttachment();
+    QCOMPARE(previewWidget->width(), 0);
 
+    // Show the preview widget
+    QTest::mouseClick(m_textWidget->findChild<QPushButton*>("previewPushButton"), Qt::LeftButton);
+    QCoreApplication::processEvents();
+    QVERIFY(previewWidget->width() > 0);
+
+    // Verify attachment data has been loaded in the preview
+    attachments = previewWidget->getAttachment();
     QCOMPARE(attachments.name, Test.name);
     QCOMPARE(attachments.data, Test.data);
 }