Fix font comparisons

When inheritance of explicitly set font attributes is involved, it is
important to compare the resolve mask in addition to comparing the
attribute values to avoid losing the information whether a specific
attribute is set explicitly.

For example, QFont::operator==() returns true for two fonts that
are both bold regardless of whether only one of them has explicit
weight set. Therefore, setFont() must not ignore fonts resolve mask.

Furthermore, the fontChanged() notifier must be emitted only if the
actual attribute values change, not if only the resolve mask changes.

Change-Id: I2eb7a071c0eaecd2f8d2f6074c4ce6ed6764a6e9
Task-number: QTBUG-50984
Task-number: QTBUG-51696
Reviewed-by: J-P Nurmi <jpnurmi@theqtcompany.com>
Reviewed-by: Liang Qi <liang.qi@theqtcompany.com>

9 files changed, 227 insertions, 13 deletions

diff --git a/src/templates/qquickapplicationwindow.cpp b/src/templates/qquickapplicationwindow.cpp
index 9a9e5460..309c8d3a 100644
--- a/src/templates/qquickapplicationwindow.cpp
+++ b/src/templates/qquickapplicationwindow.cpp
@@ -396,7 +396,7 @@ QFont QQuickApplicationWindow::font() const
 void QQuickApplicationWindow::setFont(const QFont &f)
 {
     Q_D(QQuickApplicationWindow);
-    if (d->font == f)
+    if (d->font.resolve() == f.resolve() && d->font == f)
         return;
 
     QFont resolvedFont = f.resolve(QQuickControlPrivate::themeFont(QPlatformTheme::SystemFont));
@@ -417,11 +417,13 @@ void QQuickApplicationWindowPrivate::resolveFont()
 void QQuickApplicationWindowPrivate::updateFont(const QFont &f)
 {
     Q_Q(QQuickApplicationWindow);
+    const bool changed = font != f;
     font = f;
 
     QQuickControlPrivate::updateFontRecur(q->contentItem(), f);
 
-    emit q->fontChanged();
+    if (changed)
+        emit q->fontChanged();
 }
 
 QLocale QQuickApplicationWindow::locale() const
diff --git a/src/templates/qquickcontrol.cpp b/src/templates/qquickcontrol.cpp
index 0e14bc37..78ecaa91 100644
--- a/src/templates/qquickcontrol.cpp
+++ b/src/templates/qquickcontrol.cpp
@@ -287,11 +287,13 @@ void QQuickControlPrivate::resolveFont()
 void QQuickControlPrivate::updateFont(const QFont &f)
 {
     Q_Q(QQuickControl);
+    const bool changed = font != f;
     font = f;
 
     QQuickControlPrivate::updateFontRecur(q, f);
 
-    emit q->fontChanged();
+    if (changed)
+        emit q->fontChanged();
 }
 
 void QQuickControlPrivate::updateFontRecur(QQuickItem *item, const QFont &f)
@@ -411,7 +413,7 @@ QFont QQuickControl::font() const
 void QQuickControl::setFont(const QFont &f)
 {
     Q_D(QQuickControl);
-    if (d->font == f)
+    if (d->font.resolve() == f.resolve() && d->font == f)
         return;
 
     // Determine which font is inherited from this control's ancestors and
diff --git a/src/templates/qquicklabel.cpp b/src/templates/qquicklabel.cpp
index 13ff3a6e..7de520d9 100644
--- a/src/templates/qquicklabel.cpp
+++ b/src/templates/qquicklabel.cpp
@@ -113,9 +113,12 @@ void QQuickLabelPrivate::resolveFont()
     if (sourceFont.resolve() == resolvedFont.resolve() && sourceFont == resolvedFont)
         return;
 
+    const bool changed = sourceFont != resolvedFont;
+
     q->QQuickText::setFont(resolvedFont);
 
-    emit q->fontChanged();
+    if (changed)
+        emit q->fontChanged();
 }
 
 void QQuickLabelPrivate::_q_textChanged(const QString &text)
@@ -158,7 +161,7 @@ QFont QQuickLabel::font() const
 void QQuickLabel::setFont(const QFont &font)
 {
     Q_D(QQuickLabel);
-    if (d->sourceFont == font)
+    if (d->sourceFont.resolve() == font.resolve() && d->sourceFont == font)
         return;
 
     // Determine which font is inherited from this control's ancestors and
@@ -170,9 +173,12 @@ void QQuickLabel::setFont(const QFont &font)
     if (d->sourceFont.resolve() == resolvedFont.resolve() && d->sourceFont == resolvedFont)
         return;
 
+    const bool changed = d->sourceFont != resolvedFont;
+
     QQuickText::setFont(font);
 
-    emit fontChanged();
+    if (changed)
+        emit fontChanged();
 }
 
 /*!
diff --git a/src/templates/qquicktextarea.cpp b/src/templates/qquicktextarea.cpp
index 05b1b9a7..97eff350 100644
--- a/src/templates/qquicktextarea.cpp
+++ b/src/templates/qquicktextarea.cpp
@@ -166,9 +166,12 @@ void QQuickTextAreaPrivate::resolveFont()
     if (sourceFont.resolve() == resolvedFont.resolve() && sourceFont == resolvedFont)
         return;
 
+    const bool changed = sourceFont != resolvedFont;
+
     q->QQuickTextEdit::setFont(resolvedFont);
 
-    emit q->fontChanged();
+    if (changed)
+        emit q->fontChanged();
 }
 
 void QQuickTextAreaPrivate::_q_readOnlyChanged(bool isReadOnly)
@@ -212,7 +215,7 @@ QFont QQuickTextArea::font() const
 void QQuickTextArea::setFont(const QFont &font)
 {
     Q_D(QQuickTextArea);
-    if (d->sourceFont == font)
+    if (d->sourceFont.resolve() == font.resolve() && d->sourceFont == font)
         return;
 
     // Determine which font is inherited from this control's ancestors and
@@ -224,9 +227,12 @@ void QQuickTextArea::setFont(const QFont &font)
     if (d->sourceFont.resolve() == resolvedFont.resolve() && d->sourceFont == resolvedFont)
         return;
 
+    const bool changed = d->sourceFont != resolvedFont;
+
     QQuickTextEdit::setFont(font);
 
-    emit fontChanged();
+    if (changed)
+        emit fontChanged();
 }
 
 /*!
diff --git a/src/templates/qquicktextfield.cpp b/src/templates/qquicktextfield.cpp
index c3070614..06a2f8e0 100644
--- a/src/templates/qquicktextfield.cpp
+++ b/src/templates/qquicktextfield.cpp
@@ -180,9 +180,12 @@ void QQuickTextFieldPrivate::resolveFont()
     if (sourceFont.resolve() == resolvedFont.resolve() && sourceFont == resolvedFont)
         return;
 
+    const bool changed = sourceFont != resolvedFont;
+
     q->QQuickTextInput::setFont(resolvedFont);
 
-    emit q->fontChanged();
+    if (changed)
+        emit q->fontChanged();
 }
 
 void QQuickTextFieldPrivate::_q_readOnlyChanged(bool isReadOnly)
@@ -237,7 +240,7 @@ QFont QQuickTextField::font() const
 void QQuickTextField::setFont(const QFont &font)
 {
     Q_D(QQuickTextField);
-    if (d->sourceFont == font)
+    if (d->sourceFont.resolve() == font.resolve() && d->sourceFont == font)
         return;
 
     // Determine which font is inherited from this control's ancestors and
@@ -249,9 +252,12 @@ void QQuickTextField::setFont(const QFont &font)
     if (d->sourceFont.resolve() == resolvedFont.resolve() && d->sourceFont == resolvedFont)
         return;
 
+    const bool changed = d->sourceFont != resolvedFont;
+
     QQuickTextInput::setFont(font);
 
-    emit fontChanged();
+    if (changed)
+        emit fontChanged();
 }
 
 /*!
diff --git a/tests/auto/controls/data/tst_control.qml b/tests/auto/controls/data/tst_control.qml
index 7fe61064..dd9abefe 100644
--- a/tests/auto/controls/data/tst_control.qml
+++ b/tests/auto/controls/data/tst_control.qml
@@ -56,6 +56,11 @@ TestCase {
         T.Control { }
     }
 
+    Component {
+        id: signalSpy
+        SignalSpy { }
+    }
+
     SignalSpy {
         id: mirroredSpy
         signalName: "mirroredChanged"
@@ -626,6 +631,49 @@ TestCase {
         control4.destroy()
     }
 
+    function test_font_explicit_attributes_data() {
+        return [
+            {tag: "bold", value: true},
+            {tag: "capitalization", value: Font.Capitalize},
+            {tag: "family", value: "Courier"},
+            {tag: "italic", value: true},
+            {tag: "strikeout", value: true},
+            {tag: "underline", value: true},
+            {tag: "weight", value: Font.Black},
+            {tag: "wordSpacing", value: 55}
+        ]
+    }
+
+    function test_font_explicit_attributes(data) {
+        var control = component.createObject(testCase)
+        verify(control)
+
+        var child = component.createObject(control)
+        verify(child)
+
+        var controlSpy = signalSpy.createObject(control, {target: control, signalName: "fontChanged"})
+        verify(controlSpy.valid)
+
+        var childSpy = signalSpy.createObject(child, {target: child, signalName: "fontChanged"})
+        verify(childSpy.valid)
+
+        var defaultValue = control.font[data.tag]
+        child.font[data.tag] = defaultValue
+
+        compare(child.font[data.tag], defaultValue)
+        compare(childSpy.count, 0)
+
+        control.font[data.tag] = data.value
+
+        compare(control.font[data.tag], data.value)
+        compare(controlSpy.count, 1)
+
+        compare(child.font[data.tag], defaultValue)
+        compare(childSpy.count, 0)
+
+        control.destroy()
+    }
+
     function test_locale() {
         var control = component.createObject(testCase)
         verify(control)
diff --git a/tests/auto/controls/data/tst_label.qml b/tests/auto/controls/data/tst_label.qml
index 2af02cdf..b93ca3c7 100644
--- a/tests/auto/controls/data/tst_label.qml
+++ b/tests/auto/controls/data/tst_label.qml
@@ -55,9 +55,57 @@ TestCase {
         Label { }
     }
 
+    Component {
+        id: signalSpy
+        SignalSpy { }
+    }
+
     function test_creation() {
         var control = label.createObject(testCase)
         verify(control)
         control.destroy()
     }
+
+    function test_font_explicit_attributes_data() {
+        return [
+            {tag: "bold", value: true},
+            {tag: "capitalization", value: Font.Capitalize},
+            {tag: "family", value: "Courier"},
+            {tag: "italic", value: true},
+            {tag: "strikeout", value: true},
+            {tag: "underline", value: true},
+            {tag: "weight", value: Font.Black},
+            {tag: "wordSpacing", value: 55}
+        ]
+    }
+
+    function test_font_explicit_attributes(data) {
+        var control = label.createObject(testCase)
+        verify(control)
+
+        var child = label.createObject(control)
+        verify(child)
+
+        var controlSpy = signalSpy.createObject(control, {target: control, signalName: "fontChanged"})
+        verify(controlSpy.valid)
+
+        var childSpy = signalSpy.createObject(child, {target: child, signalName: "fontChanged"})
+        verify(childSpy.valid)
+
+        var defaultValue = control.font[data.tag]
+        child.font[data.tag] = defaultValue
+
+        compare(child.font[data.tag], defaultValue)
+        compare(childSpy.count, 0)
+
+        control.font[data.tag] = data.value
+
+        compare(control.font[data.tag], data.value)
+        compare(controlSpy.count, 1)
+
+        compare(child.font[data.tag], defaultValue)
+        compare(childSpy.count, 0)
+
+        control.destroy()
+    }
 }
diff --git a/tests/auto/controls/data/tst_textarea.qml b/tests/auto/controls/data/tst_textarea.qml
index b1225493..269e11ec 100644
--- a/tests/auto/controls/data/tst_textarea.qml
+++ b/tests/auto/controls/data/tst_textarea.qml
@@ -55,6 +55,11 @@ TestCase {
         TextArea { background: Item { } }
     }
 
+    Component {
+        id: signalSpy
+        SignalSpy { }
+    }
+
     function test_creation() {
         var control = textArea.createObject(testCase)
         verify(control)
@@ -89,4 +94,47 @@ TestCase {
 
         control.destroy()
     }
+
+    function test_font_explicit_attributes_data() {
+        return [
+            {tag: "bold", value: true},
+            {tag: "capitalization", value: Font.Capitalize},
+            {tag: "family", value: "Courier"},
+            {tag: "italic", value: true},
+            {tag: "strikeout", value: true},
+            {tag: "underline", value: true},
+            {tag: "weight", value: Font.Black},
+            {tag: "wordSpacing", value: 55}
+        ]
+    }
+
+    function test_font_explicit_attributes(data) {
+        var control = textArea.createObject(testCase)
+        verify(control)
+
+        var child = textArea.createObject(control)
+        verify(child)
+
+        var controlSpy = signalSpy.createObject(control, {target: control, signalName: "fontChanged"})
+        verify(controlSpy.valid)
+
+        var childSpy = signalSpy.createObject(child, {target: child, signalName: "fontChanged"})
+        verify(childSpy.valid)
+
+        var defaultValue = control.font[data.tag]
+        child.font[data.tag] = defaultValue
+
+        compare(child.font[data.tag], defaultValue)
+        compare(childSpy.count, 0)
+
+        control.font[data.tag] = data.value
+
+        compare(control.font[data.tag], data.value)
+        compare(controlSpy.count, 1)
+
+        compare(child.font[data.tag], defaultValue)
+        compare(childSpy.count, 0)
+
+        control.destroy()
+    }
 }
diff --git a/tests/auto/controls/data/tst_textfield.qml b/tests/auto/controls/data/tst_textfield.qml
index 57675659..a7e2f6ec 100644
--- a/tests/auto/controls/data/tst_textfield.qml
+++ b/tests/auto/controls/data/tst_textfield.qml
@@ -55,6 +55,11 @@ TestCase {
         TextField { }
     }
 
+    Component {
+        id: signalSpy
+        SignalSpy { }
+    }
+
     function test_creation() {
         var control = textField.createObject(testCase)
         verify(control)
@@ -90,4 +95,47 @@ TestCase {
 
         control.destroy()
     }
+
+    function test_font_explicit_attributes_data() {
+        return [
+            {tag: "bold", value: true},
+            {tag: "capitalization", value: Font.Capitalize},
+            {tag: "family", value: "Courier"},
+            {tag: "italic", value: true},
+            {tag: "strikeout", value: true},
+            {tag: "underline", value: true},
+            {tag: "weight", value: Font.Black},
+            {tag: "wordSpacing", value: 55}
+        ]
+    }
+
+    function test_font_explicit_attributes(data) {
+        var control = textField.createObject(testCase)
+        verify(control)
+
+        var child = textField.createObject(control)
+        verify(child)
+
+        var controlSpy = signalSpy.createObject(control, {target: control, signalName: "fontChanged"})
+        verify(controlSpy.valid)
+
+        var childSpy = signalSpy.createObject(child, {target: child, signalName: "fontChanged"})
+        verify(childSpy.valid)
+
+        var defaultValue = control.font[data.tag]
+        child.font[data.tag] = defaultValue
+
+        compare(child.font[data.tag], defaultValue)
+        compare(childSpy.count, 0)
+
+        control.font[data.tag] = data.value
+
+        compare(control.font[data.tag], data.value)
+        compare(controlSpy.count, 1)
+
+        compare(child.font[data.tag], defaultValue)
+        compare(childSpy.count, 0)
+
+        control.destroy()
+    }
 }