From 97499850892d91be74894ce38e30e096b5b322d8 Mon Sep 17 00:00:00 2001 From: ispyisail Date: Sat, 3 Oct 2026 10:18:06 +1300 Subject: [PATCH] Fix #1238: cross-reference drawn over the label in older projects XRefProperties read the stored cross-reference position with QMetaEnum::keyToValue() and cast the result straight to Qt::AlignmentFlag. An empty value gives -1. Earlier versions saved xrefpos="" into projects and the settings file (18 of the 29 shipped examples that carry cross-reference settings have it), and -1 matches no branch of DynamicElementTextItem::setXref_item(), so the cross-reference stayed at (0,0): the top-left corner of the label, on top of it. fromXml() fell back to AlignBottom only when the attribute was missing, and fromSettings() only when the key was missing (#296), so an empty value kept producing the bad position and was saved back empty on every save. Both now go through one helper that returns AlignBottom for an empty, unknown, or not-offered value. The next save writes "AlignBottom", so affected projects heal once resaved. tst_xrefpos covers fromXml(), fromSettings() and the rewrite on save; it fails 5 of 14 cases without this change. Exporting examples/2612_ats_singlephase.qet to SVG before and after shows every slave cross-reference moving from over its label to below it. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01ELKpbGxqJd7EFiTUypBtVe --- sources/properties/xrefproperties.cpp | 42 +++++++++--- tests/qttest/CMakeLists.txt | 12 ++++ tests/qttest/tst_xrefpos.cpp | 94 +++++++++++++++++++++++++++ 3 files changed, 138 insertions(+), 10 deletions(-) create mode 100644 tests/qttest/tst_xrefpos.cpp diff --git a/sources/properties/xrefproperties.cpp b/sources/properties/xrefproperties.cpp index 601fb02ef..22fe44cea 100644 --- a/sources/properties/xrefproperties.cpp +++ b/sources/properties/xrefproperties.cpp @@ -22,6 +22,36 @@ #include #include +namespace { +/** + @brief xrefPosFromKey + @param key : stored name of the position, e.g. "AlignBottom" + @return the position, or Qt::AlignBottom if key is empty or not one of + the positions the cross-reference settings offer. Older versions saved + an empty "xrefpos", which drew the cross-reference over the label. +*/ +Qt::AlignmentFlag xrefPosFromKey(const QString &key) +{ + bool ok = false; + const int value = QMetaEnum::fromType() + .keyToValue(key.toStdString().data(), &ok); + if (!ok) + return Qt::AlignBottom; + + switch (value) { + case Qt::AlignBottom: + case Qt::AlignTop: + case Qt::AlignLeft: + case Qt::AlignRight: + case Qt::AlignBaseline: + case Qt::AlignHCenter: + return Qt::AlignmentFlag(value); + default: + return Qt::AlignBottom; + } +} +} + /** @brief XRefProperties::XRefProperties Default Constructor @@ -96,8 +126,7 @@ void XRefProperties::fromSettings(const QSettings &settings, m_master_label = settings.value(prefix % "master_label", "%f-%l%c").toString(); m_slave_label = settings.value(prefix % "slave_label", "(%f-%l%c)").toString(); - QMetaEnum var = QMetaEnum::fromType(); - m_xref_pos = Qt::AlignmentFlag(var.keyToValue((settings.value(prefix % "xrefpos", "AlignBottom").toString()).toStdString().data())); + m_xref_pos = xrefPosFromKey(settings.value(prefix % "xrefpos").toString()); for (QString key : m_prefix_keys) { m_prefix.insert(key, settings.value(prefix + key % "prefix").toString()); @@ -157,14 +186,7 @@ bool XRefProperties::fromXml(const QDomElement &xml_element) { QString snap = xml_element.attribute("snapto", "label"); snap == "bottom"? m_snap_to = Bottom : m_snap_to = Label; - QString xrefpos = xml_element.attribute("xrefpos","Left"); - - QMetaEnum var = QMetaEnum::fromType(); - - if(xml_element.hasAttribute("xrefpos")) - m_xref_pos = Qt::AlignmentFlag(var.keyToValue(xml_element.attribute("xrefpos").toStdString().data())); - else - m_xref_pos = Qt::AlignBottom; + m_xref_pos = xrefPosFromKey(xml_element.attribute("xrefpos")); m_offset = xml_element.attribute("offset", "0").toInt(); m_slave_offset = xml_element.attribute("slave_offset", "0").toInt(); diff --git a/tests/qttest/CMakeLists.txt b/tests/qttest/CMakeLists.txt index 41951877e..1303cdbdb 100644 --- a/tests/qttest/CMakeLists.txt +++ b/tests/qttest/CMakeLists.txt @@ -642,3 +642,15 @@ add_executable( add_test(NAME tst_diagramcontext COMMAND tst_diagramcontext) target_include_directories(tst_diagramcontext PRIVATE ${QET_DIR}/sources) target_link_libraries(tst_diagramcontext PRIVATE Qt::Test Qt::Widgets Qt::Xml pugixml::pugixml) + +# XRefProperties: an empty or unknown cross-reference position, as older +# versions saved it, reads as the default instead of drawing the +# cross-reference over the element's label (issue #1238). +add_executable( + tst_xrefpos + tst_xrefpos.cpp + ${QET_DIR}/sources/properties/xrefproperties.cpp + ${QET_DIR}/sources/properties/propertiesinterface.cpp) +add_test(NAME tst_xrefpos COMMAND tst_xrefpos) +target_include_directories(tst_xrefpos PRIVATE ${QET_DIR}/sources) +target_link_libraries(tst_xrefpos PRIVATE Qt::Test Qt::Widgets Qt::Xml pugixml::pugixml) diff --git a/tests/qttest/tst_xrefpos.cpp b/tests/qttest/tst_xrefpos.cpp new file mode 100644 index 000000000..6e57016fe --- /dev/null +++ b/tests/qttest/tst_xrefpos.cpp @@ -0,0 +1,94 @@ +// SPDX-License-Identifier: GPL-2.0-or-later +#include + +#include "properties/xrefproperties.h" + +/** + XRefProperties reads the cross-reference position from a project's + and from the settings file. Older versions saved an + empty value; read as-is it drew the cross-reference over the element's + label (issue #1238). An empty or unknown value must fall back to the + default, AlignBottom, and a valid one must be kept. +*/ +class tst_xrefpos : public QObject +{ + Q_OBJECT + + static Qt::AlignmentFlag fromXml(const QString &attribute, bool present = true) + { + QDomDocument doc; + QDomElement e = doc.createElement("xref"); + e.setAttribute("type", "protection"); + if (present) + e.setAttribute("xrefpos", attribute); + XRefProperties xrp; + xrp.fromXml(e); + return xrp.getXrefPos(); + } + + private slots: + void xml_data() + { + QTest::addColumn("stored"); + QTest::addColumn("expected"); + QTest::newRow("empty") << "" << int(Qt::AlignBottom); + QTest::newRow("garbage") << "Sideways" << int(Qt::AlignBottom); + QTest::newRow("not offered") << "AlignJustify" << int(Qt::AlignBottom); + QTest::newRow("bottom") << "AlignBottom" << int(Qt::AlignBottom); + QTest::newRow("top") << "AlignTop" << int(Qt::AlignTop); + QTest::newRow("left") << "AlignLeft" << int(Qt::AlignLeft); + QTest::newRow("right") << "AlignRight" << int(Qt::AlignRight); + QTest::newRow("baseline") << "AlignBaseline" << int(Qt::AlignBaseline); + QTest::newRow("hcenter") << "AlignHCenter" << int(Qt::AlignHCenter); + } + + void xml() + { + QFETCH(QString, stored); + QFETCH(int, expected); + QCOMPARE(int(fromXml(stored)), expected); + } + + void xmlMissing() + { + QCOMPARE(fromXml(QString(), false), Qt::AlignBottom); + } + + /** + An empty value is written back as "AlignBottom" on the next save, + so the file heals instead of carrying the empty value forward. + */ + void emptyIsRewritten() + { + QDomDocument doc; + QDomElement e = doc.createElement("xref"); + e.setAttribute("xrefpos", ""); + XRefProperties xrp; + xrp.fromXml(e); + QCOMPARE(xrp.toXml(doc).attribute("xrefpos"), QStringLiteral("AlignBottom")); + } + + void settings() + { + QTemporaryDir dir; + QVERIFY(dir.isValid()); + QSettings s(dir.filePath("qet.ini"), QSettings::IniFormat); + XRefProperties xrp; + + s.setValue("xrefprotectionxrefpos", ""); + xrp.fromSettings(s, "xrefprotection"); + QCOMPARE(xrp.getXrefPos(), Qt::AlignBottom); + + s.remove("xrefprotectionxrefpos"); + xrp.setXrefPos(Qt::AlignTop); + xrp.fromSettings(s, "xrefprotection"); + QCOMPARE(xrp.getXrefPos(), Qt::AlignBottom); + + s.setValue("xrefprotectionxrefpos", "AlignRight"); + xrp.fromSettings(s, "xrefprotection"); + QCOMPARE(xrp.getXrefPos(), Qt::AlignRight); + } +}; + +QTEST_GUILESS_MAIN(tst_xrefpos) +#include "tst_xrefpos.moc"