Bug 399155

Not sure if this is fixable. The number of pixels between the top and the bottom is determined by the font size and type. If we "fixed" this for one font, it would break for other fonts. There's no clean way to ensure this programmatically.

Right. The dots are actually characters, rendered via libfreetype. The only way to fix it would be in Qt to use actual circles instead of characters. But then people would complain that password substitution characters do no longer follow platform defaults.

There are widget styles where the password substitution character is configurable, at least in Skulpture it is, but I guess QtCurve allows the same.

Re-opening since I think I misunderstood the issue. The problem is that the password field gets taller when you start typing. That's still the case, definitely shouldn't happen, and should be fixable.

Created attachment 118309 [details]
Video demonstrating the issue

This video depicts me typing in the lock screen in Neon Dev Unstable, with default font settings etc. Notice how the password field gets taller when there's any text in it.

Neovar, is neon the only distribution you've tested this on?

I can reproduce this bug on neon, but not on Arch or Fedora.

Yes it only happens on neon stable, unstable.

Remember trying Kubuntu and the issue was not present it started after 5.12  probably 5.13 when the new blur and redesign on the login and lock screen debuted.

Yeah I can't reproduce it in Kubuntu either

Okay so I managed to track down the cause, it's in /etc/fonts/conf.d/56-neon-noto.conf. If you delete that file, no more bug.
I still don't know why though :D

It's something in the code in lines 80-121 - if I remove lines 85 thru 119, the bug disappears, some kind of weird substitution occurs.

Aha, I was actually just going to suggest that it might be something weird in the fontconfig after having a dream about this! Nice find.

Haha thanks... It really kept tripping me up - why Neon and not Arch, Fedora or Kubuntu even

When you remove those lines the Noto fonts are no longer used. So, this answers nothing much as this is still either a metrics calculation issue in Qt or the qml components, or a problem with the font itself. When I open NotoSymbols2 and NotoSans and compare U+2B24 (which I think is the particular type of circle) to other characters they seem to in fact have different heights. Now I don't know the next thing about font design but I'm inclined to assume that if they are different heights in fontforge, they'll be different size when rendered.

What puzzles me though is when you copy ⬤⬤⬤⬤⬤ into kickoff's lineedit it doesn't switch size. What it means or if it means anything I don't know.

In conclusion I think the problem is the NotoSymbols2 font itself. So you'll probably want to file the bug report with them on github.

Furthermore I think the problem likely would go away if we simply used U+25CF which is a regular sized black circle (U+2B24 is big black circle, so in a way it's rendering the kind of circle that was requested ;)).
Moving this bug to plasma-integration for consideration. AFAIK the platformtheme sets the password masking character.

Sure, but U+2B24 doesn't seem to result in any problems in any other distros that don't have Neon's fontconfig settings. Does this mean that other distros are actually not using the noto font for the SDDM user?

I guess we could just use the alternative U+25CF character.

Created attachment 118356 [details]
noto-1-vs-dejavu-1

(In reply to Nate Graham from comment #13)
> Does this mean that other distros are actually not using the noto font for the SDDM user?

That is correct. Well, sddm + lockscreen, I think the same font configs (Qt's) apply to both and as such they would by default be 'sans-serif' and what that means is controlled through fontconfig and that is usually not Noto (unless you are on Neon or ChromeOS at least).

An easy way to tell is looking at the date and time: Noto doesn't have a serif "1" character, while many other default font choices (DejaVu and Bitstream Vera come to mind) tend to have a serif 1.

For illustration I am attaching a screenshot of Noto 1 vs. DejaVu 1.

Oh actually, having written that, I am not sure plasma-integration can actually do anything about the password character. At least on sddm I think the platformtheme for Plasma isn't actually loaded, what with not being a plasma session and all.

All right, let's use a different character. Do you know where that gets set?

Not specifically, I don't know which QML components are involved here unfortunately. From the actual UI control the call chain ultimately goes deep into Qt (qcommonstyle I think?) and from there into the QPlatformTheme (i.e. plasma-integration for plasma sessions; but as I say that has no impact on SDDM most likely).
Chances are there is a way to influence the character from QML though.  I know that via qstylesheets you could change the character for qwidget QLabel.

I had a quick grep in qtbase and I fear this needs some investigating.

There is this (but I think that isn't it because that's actually a tiny bullet •••)
platformsupport/themes/genericunix/qgenericunixthemes.cpp-    case QPlatformTheme::PasswordMaskCharacter:
platformsupport/themes/genericunix/qgenericunixthemes.cpp:        return QVariant(QChar(0x2022));

Then there is also this, but that's already the character I was suggesting ●●●●
gui/kernel/qplatformtheme.cpp-    case QPlatformTheme::PasswordMaskCharacter:
gui/kernel/qplatformtheme.cpp:        return QVariant(QChar(0x25CF));

Beyond those two I see no relevant code inside qtbase. There could be in qtquick controls, but I don't have a clone of that at hand.

Or I am wrong and we are already using U+25CF and I am reading the fontforge window incorrectly and the glyph is in fact larger than other characters. In which case we'd have to move to 2022 (the bullet) ^^

This happens with other fonts too (Ubuntu, Inconsolata etc.) which might be because they're defaulting to Noto?

"
Or I am wrong and we are already using U+25CF and I am reading the fontforge window incorrectly and the glyph is in fact larger than other characters. In which case we'd have to move to 2022 (the bullet)"

Maybe Kickoff and other widgets just cut it off and the sddm-theme adapts to its size (makes the field bigger). In either case, a lot of other fonts (including whatever neon defaults to that isn't noto) don't do this, so I propose that we either modify Noto (can we do that?) or use a different glyph.

Yes. If a font doesn't provide a character the next higher preferred font in the same family will provide it.

tightBoundingRect() on a FontMetrics with $text should be glyph substitution aware. If not, a hack would be a hidden Text {} with the same font doing the symbol and then looking at painted/implicitHeight.

That made me think that this actually may be a problem with how the height is calculated when in password echo mode. The current input is a plasmacomponents 2 TextField, when I switch it for a qt controls TextField the height is large enough to accommodate the password character. So perhaps the problem is actually in plasmacomponents/kirigami where it should get the `max(placeholderText, passwordCharacterHeight)`?

For the record: the font metrics are indeed different

Noto Sans
Ascent: 12
Descent: 4
Height: 16

Noto Sans Symbols2
Ascent: 12
Descent: 7
Height: 19

(all values from FontMetrics component)

Shouldn't noto sans symbols2 be fixed (too) then?

It's probably worth filing a bug with Noto. The size difference seems intentional though, so there may well be a rationale for why Symbols2 has larger metrics.

You might find PlasmaComponents3.TextField doesn't jump about. 

The height code is much more sensible.

(In reply to David Edmundson from comment #26)
> You might find PlasmaComponents3.TextField doesn't jump about. 
> 
> The height code is much more sensible.

Good call - it doesn't (it even looks leaner), but this happens
https://phabricator.kde.org/file/data/x75du5yanhince3tbuy4/PHID-FILE-q7ao2tbeicsq6s3q62k7/image.png

Which I'm assuming is because of noto itself?

(the issue doesn't occur with the plasmacomponents 3 textfield without the 56-neon-noto.conf file / when using a different font / different glyphs)

If you set the font.family to "Noto Sans Symbols2" the password is permanently tall (deformed)

I strongly maintain that this is an issue with Noto Sans Symbols2 and that removing the relevant lines of code from 56-neon-noto.conf is a perfectly sound temporary workaround for this issue.

We need a solution not a workaround. Noto Sans is the default font for Plasma, its symbols causing display glitches is a problem that needs solving.

(In reply to Harald Sitter from comment #30)
> We need a solution not a workaround. Noto Sans is the default font for
> Plasma, its symbols causing display glitches is a problem that needs solving.

Noto Sans is indeed the default font in Plasma. Noto Sans Symbols2 is not.
And there are only so many ways to render a circle.

I never said we didn't need a solution. I agree that we do but given Noto Sans' other failings, the fact that no font is ever perfect (and that we shouldn't cling to Noto Sans so fervently), and the fact that the circles as rendered by a different font whose height matches that of Noto Sans (and not Symbols2) look the same as those of Noto Sans Symbols2, this workaround seems like an acceptable provisional solution.

We can put back the config file after the height issue is resolved.

Here is a minimal code to reproduce the bug:

###########

import QtQuick 2.0
import QtQuick.Layouts 1.2
import org.kde.plasma.components 2.0 as PlasmaComponents

Rectangle {
    width: passwordBox.width + 4
    height: passwordBox.height + 4
    color: "grey"

    PlasmaComponents.TextField {
        id: passwordBox
        revealPasswordButtonShown: true
        echoMode: TextInput.Password
        Layout.fillWidth: true
        font.family: "Noto Color Emoji"
    }

}

###########
When run it with qmlscene:

qmlscene test.qml
file:///usr/lib64/qt5/qml/QtQuick/Controls/Styles/Plasma/TextFieldStyle.qml:53:17: QML QQuickItem: Binding loop detected for property "implicitHeight"
file:///usr/lib64/qt5/qml/QtQuick/Controls/Styles/Plasma/TextFieldStyle.qml:53:17: QML QQuickItem: Binding loop detected for property "implicitHeight"
file:///usr/lib64/qt5/qml/QtQuick/Controls/Styles/Plasma/TextFieldStyle.qml:53:17: QML QQuickItem: Binding loop detected for property "implicitHeight"
file:///usr/lib64/qt5/qml/QtQuick/Controls/Styles/Plasma/TextFieldStyle.qml:53:17: QML QQuickItem: Binding loop detected for property "implicitHeight"

(In reply to trmdi from comment #32)
> Here is a minimal code to reproduce the bug:
> 
> ###########
> 
> import QtQuick 2.0
> import QtQuick.Layouts 1.2
> import org.kde.plasma.components 2.0 as PlasmaComponents
> 
> Rectangle {
>     width: passwordBox.width + 4
>     height: passwordBox.height + 4
>     color: "grey"
> 
>     PlasmaComponents.TextField {
>         id: passwordBox
>         revealPasswordButtonShown: true
>         echoMode: TextInput.Password
>         Layout.fillWidth: true
>         font.family: "Noto Color Emoji"
>     }
> 
> }
> 
> ###########
> When run it with qmlscene:
> 
> qmlscene test.qml
> file:///usr/lib64/qt5/qml/QtQuick/Controls/Styles/Plasma/TextFieldStyle.qml:
> 53:17: QML QQuickItem: Binding loop detected for property "implicitHeight"
> file:///usr/lib64/qt5/qml/QtQuick/Controls/Styles/Plasma/TextFieldStyle.qml:
> 53:17: QML QQuickItem: Binding loop detected for property "implicitHeight"
> file:///usr/lib64/qt5/qml/QtQuick/Controls/Styles/Plasma/TextFieldStyle.qml:
> 53:17: QML QQuickItem: Binding loop detected for property "implicitHeight"
> file:///usr/lib64/qt5/qml/QtQuick/Controls/Styles/Plasma/TextFieldStyle.qml:
> 53:17: QML QQuickItem: Binding loop detected for property "implicitHeight"

Yes that works. You could also use just "Noto Sans" instead of the emoji font.

If I set the height value to accommodate Noto Sans (I haven't figured out how to resolve the binding loop yet so I used a ballpark estimate), the circles get cut off again (not unlike the PlasmaComponents 3 screenshot: https://phabricator.kde.org/file/data/x75du5yanhince3tbuy4/PHID-FILE-q7ao2tbeicsq6s3q62k7/image.png).

While this issue needs to be resolved too, it's not going to be enough.

P.S. @trmdi you could also try the PlasmaComponents 3 text field (no binding loop) for reference

(In reply to Krešimir Čohar from comment #33)
I haven't figured out
> how to resolve the binding loop yet...

Maybe by using FontMetrics.height instead of control.cursorRectangle.height in /usr/lib64/qt5/qml/QtQuick/Controls/Styles/Plasma/TextFieldStyle.qml line 63.
But we also have to use QtQuick 2.12 instead.

(In reply to trmdi from comment #35)
> (In reply to Krešimir Čohar from comment #33)
> I haven't figured out
> > how to resolve the binding loop yet...
> 
> Maybe by using FontMetrics.height instead of control.cursorRectangle.height
> in /usr/lib64/qt5/qml/QtQuick/Controls/Styles/Plasma/TextFieldStyle.qml line
> 63.
> But we also have to use QtQuick 2.12 instead.

Good idea. That eliminates the loop.
But it also leaves the field kind of short (Noto Sans gets cut off at the bottom and Noto Sans Symbols2 gets cut off at the top).

And yeah I had to bump QtQuick to make it work.

But I think you're on the right track.

(In reply to Krešimir Čohar from comment #36)
> But it also leaves the field kind of short (Noto Sans gets cut off at the
> bottom and Noto Sans Symbols2 gets cut off at the top).

Try: 

FontMetrics {
    ...
    font.pixelSize: control.font.pixelSize
    ...
}

After much investigation by Marco and Harald (thanks co much guys!), this turned out to have been caused by two distinct issues that interact poorly with one another:

- PlasmaComponents2 Text field inappropriately gets taller to accommodate the text, rather than making the text fit in the text field. This is fixed with https://phabricator.kde.org/D19599

- The Noto Symbols2 font has an incorrect Descent value (7 va 4 in Noto Sans). This is tracked with https://github.com/googlei18n/noto-fonts/issues/1468

(In reply to trmdi from comment #37)
> (In reply to Krešimir Čohar from comment #36)
> > But it also leaves the field kind of short (Noto Sans gets cut off at the
> > bottom and Noto Sans Symbols2 gets cut off at the top).
> 
> Try: 
> 
> FontMetrics {
>     ...
>     font.pixelSize: control.font.pixelSize
>     ...
> }

Sorry, just ignore this.

Git commit a2749dc9c71809c9ce3566d741cf133b5bf7e046 by Marco Martin.
Committed on 08/03/2019 at 14:40.
Pushed by mart into branch 'master'.

textfield height based only on clear text

Summary:
as the implicit height could change when switching to password mode,
use a textmetrics which always computes it from the visible text

Test Plan: height doesn't change

Reviewers: #plasma, ngraham, rooty

Reviewed By: ngraham, rooty

Subscribers: sitter, rooty, ngraham, kde-frameworks-devel

Tags: #frameworks

Differential Revision: https://phabricator.kde.org/D19599

M  +9    -2    src/declarativeimports/plasmastyle/TextFieldStyle.qml

https://commits.kde.org/plasma-framework/a2749dc9c71809c9ce3566d741cf133b5bf7e046

*** Bug 405331 has been marked as a duplicate of this bug. ***

*** Bug 406402 has been marked as a duplicate of this bug. ***

Git commit 5014b50b719f7ab9837520799b5d9cde2a8b5c46 by Jan Grulich.
Committed on 11/04/2019 at 06:07.
Pushed by grulich into branch 'master'.

No need to set height to implicitHeight

Summary:
There seem to bug a bug in PlasmaComponents.TextField causing to use different
height depending on missing symbol for echo mode when the symbol is used from
a fallback font. While this was fixed in plasma-framework, we seem to be hitting
same issue when we set height to implicitHeight. It shouldn't be necessary to
set the height.

Reviewers: #plasma, davidedmundson

Reviewed By: #plasma, davidedmundson

Subscribers: plasma-devel

Tags: #plasma

Differential Revision: https://phabricator.kde.org/D20436

M  +1    -1    applet/contents/ui/PasswordField.qml

https://commits.kde.org/plasma-nm/5014b50b719f7ab9837520799b5d9cde2a8b5c46