Bug 420287 - KFontChooser sometimes selects a non-regular style by default
Summary: KFontChooser sometimes selects a non-regular style by default
Status: RESOLVED FIXED
Alias: None
Product: frameworks-kwidgetsaddons
Classification: Frameworks and Libraries
Component: general (show other bugs)
Version: unspecified
Platform: Other Linux
: VHI normal
Target Milestone: ---
Assignee: Christoph Feck
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2020-04-19 05:09 UTC by Noah Davis
Modified: 2020-08-10 14:44 UTC (History)
4 users (show)

See Also:
Latest Commit:
Version Fixed In: 5.70


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Noah Davis 2020-04-19 05:09:23 UTC
SUMMARY
KFontChooser will sometimes select a non-regular style by default even though the "Regular" style is available. This usually happens when a font family has a lot of different and unusual styles. This can cause users to accidentally select a style that isn't intended for contexts where "Regular" would be more appropriate.

STEPS TO REPRODUCE
1. Select the "Noto Sans" font family in a KFontChooser widget

OBSERVED RESULT
The default selected style is "SemiCondensed"

EXPECTED RESULT
The default selected style should be "Regular"

SOFTWARE/OS VERSIONS
Operating System: openSUSE Tumbleweed 20200415
KDE Plasma Version: 5.18.80 (git master)
KDE Frameworks Version: 5.70 (git master)
Qt Version: 5.14.1
Comment 1 Ahmad Samir 2020-04-19 11:05:32 UTC
Git commit b52622993b45a61c4060d19517a5c3b833b3b854 by Ahmad Samir.
Committed on 19/04/2020 at 11:05.
Pushed by ahmadsamir into branch 'master'.

[KFontChooser] Make styleIdentifier() more precise by adding font styleName

Summary:
styleIdentifier() tries to get the correct font style based on weight,
style, and stretch; this "styleID" could be the same for different font
styles in the same family, and it worked in most cases because "Regular"-like
styles usually are ordered first in the family styles list from QFontDatabase.
However this breaks for font families that provide many font styles, e.g.
Noto Sans, where "SemiCondensed" comes first in the list.

Make it more precise by adding the font "styleName" property to the
"styleID" we concatenate, this makes it more precise. Also handle empty
styleName font prop. (we strip it on purpose, see KConfigGroupGui::writeEntryGui()
for more details), try to pick the correct one so that we highlight
it in the font style list view.

Test Plan:
kfontchooserdialogtest app, with the initial font set to "Noto Sans",
before this change it'd select "SemiCondensed"; now it selects "Regular"
as expected.
FIXED-IN: 5.70

Reviewers: #frameworks, dfaure, cfeck, bport

Reviewed By: dfaure

Subscribers: ndavis, kde-frameworks-devel

Tags: #frameworks

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

M  +27   -2    src/kfontchooser.cpp
M  +1    -1    tests/kfontchooserdialogtest.cpp

https://commits.kde.org/kwidgetsaddons/b52622993b45a61c4060d19517a5c3b833b3b854
Comment 2 Nate Graham 2020-04-23 20:30:23 UTC
I'm not seeing that the above commit fixed the issue. Opening the Fonts dialog in the Fonts KCM still results in "SemiCondensed" being set by default, and if you don't change it, you'll get very narrow fonts when you click OK.
Comment 3 Ahmad Samir 2020-05-05 13:00:45 UTC
Git commit 0325d698181055cdaaa93b24ee80172132822d35 by Ahmad Samir.
Committed on 05/05/2020 at 13:00.
Pushed by ahmadsamir into branch 'master'.

[Fonts KCM] Remove redundant nearestExistingFont()

Summary:
It seems that nearestExistingFont() gets the same result as
`fc-match font-name`, which is basically the sans serif default font
on the system; this seems redundant as KFontChooser will fallback to
selecting the first font family in the list if the initial font it
was called with doesn't exist _and_ KFontChooser always puts "Sans Serif",
"Serif" and "Monospace" at the top of the list.

Removing nearestExistingFont() solves the issue in bug 420287 as
the font it returns will have the styleName property set, after we
went to so much trouble to clear that property so that setBold() can
work.

Test Plan:
Before the patch:
- In kdeglobals [General] set:
  fixed=Blah Mono,12,-1,5,50,0,0,0,0,0
  toolBarFont=Bogus Serif,12,-1,5,50,0,0,0,0,0
- Load the fonts KCM, and open the font dialog for Fixed and Toolbar,
  in both cases the default "sans serif" font on the system is selected,
  in my case it's "DejaVu Sans"

Apply the patch and repeat, the "Sans Serif" entry is selected, which is
an alias to "DejaVu Sans" on my system.

Reviewers: #plasma, bport, ngraham

Reviewed By: bport, ngraham

Subscribers: ngraham, plasma-devel

Tags: #plasma

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

M  +0    -66   kcms/fonts/fonts.cpp

https://commits.kde.org/plasma-desktop/0325d698181055cdaaa93b24ee80172132822d35
Comment 4 Nate Graham 2020-05-05 15:44:15 UTC
Is there anything left to fix here? It's working for me now.
Comment 5 Ahmad Samir 2020-05-05 15:54:13 UTC
Yeah, should be fixed now.
Comment 6 Nate Graham 2020-05-05 16:12:31 UTC
Great, thanks!
Comment 7 Nate Graham 2020-05-13 13:26:39 UTC
Given the behavioral change in a framework, perhaps the Plasma patch should be on the stable branch too, or else people will still get this broken until Plasma 5.19 is released.
Comment 8 Ahmad Samir 2020-05-15 07:57:52 UTC
Updating the report here, Plasma 5.18 still uses QFontDialog, so this bug report isn't directly related. However if the ",Regular" bit is removed from kdeglobals, QFontDialog will probably select the wrong style sometimes.
Comment 9 Alexander Shchadilov 2020-08-02 18:56:11 UTC
(In reply to Ahmad Samir from comment #8)
> Updating the report here, Plasma 5.18 still uses QFontDialog, so this bug
> report isn't directly related. However if the ",Regular" bit is removed from
> kdeglobals, QFontDialog will probably select the wrong style sometimes.

Does this mean 5.18 LTS branch will never be fixed? If yes, could you provide more information for a workaround, like where in ~/.config/kdeglobals ",Regular" should be placed?
I can reproduce the following behaviour in Kubuntu 20.04 (5.18.4) and openSUSE Leap 15.2 (5.18.5):

1. Place *.ttf file of Noto Sans SemiCondenced or ExtraCondenced to ~/.fonts if neither are presented in the system.
2. Open "System Settings"/"Fonts".
3. Change "General" font to Noto Sans Regular with any size other than default size.
4. Press "Apply" in "System Settings"/"Fonts".
5. Close "System Settings".

After this after every restart of "System Settings"/"Fonts" font settings for "General", "Fixed width", "Small", "Toolbar" will be silently set to one of the styles from step 1 (if font family in the option is Noto Sans). This means that a user who came to settings e.g. to tweak sub-pixel rendering will involuntary change fonts' styles, you even do not need to touch font choosers for that.
Comment 10 Ahmad Samir 2020-08-04 13:19:56 UTC
@Alexander: what's the version of the Frameworks in Ubuntu and openSuse (and do both use the plasma LTS version?).

Note that there well could be a discrepancy between what the fonts KCM shows and what's actually stored in the config file ~/.config/kdeglobals.

My guess is that you have Frameworks > 5.66, which means it has the new kconfig where the style name is dropped from the font settings so that calling setBold(true) can actually work properly.
Comment 11 Alexander Shchadilov 2020-08-04 17:41:02 UTC
(In reply to Ahmad Samir from comment #10)
> @Alexander: what's the version of the Frameworks in Ubuntu and openSuse (and
> do both use the plasma LTS version?).

Kubuntu 20.04 (tested in live session from installer)
KDE Plasma Version: 5.18.4
KDE Frameworks Version: 5.68.0
Qt Version: 5.12.8

openSUSE Leap 15.2 (my main PC, not a live session)
KDE Plasma Version: 5.18.5
KDE Frameworks Version: 5.71.0
Qt Version: 5.12.7

> Note that there well could be a discrepancy between what the fonts KCM shows
> and what's actually stored in the config file ~/.config/kdeglobals.

Discrepancy exists, in a sense: user can see an asterisk in a titlebar right after opening "System Settings"/"Fonts". After clicking "Apply" changes in kdeglobals become real and applications start to use new font styles.
Strangely, "Menu" and "Window title" are not affected because SemiCondenced and ExtraCondenced styles of Noto Sans are not presented when clicking these categories.
Comment 12 Ahmad Samir 2020-08-10 10:40:00 UTC
Sorry for the delay. I think I know what the problem is, and I am working on a fix for Plasma 5.18.
Comment 13 Alexander Shchadilov 2020-08-10 11:58:13 UTC
Supposedly there is a limited number of installations affected by that, it just happened that openSUSE provide a large set of Noto Sans by default. I see that in Debian & Co. fonts are separated in fonts-noto-core and fonts-noto-extra, same in Arch with noto-fonts and noto-fonts-extra.
A workaround: click "Defaults" button right after opening "System Settings"/"Fonts", then make corrections you need, click "Apply".
Comment 14 Ahmad Samir 2020-08-10 14:44:07 UTC
Git commit e5e5f5ed51aadfac99bfbdf3d2db5be16a12443b by Ahmad Samir.
Committed on 10/08/2020 at 12:52.
Pushed by ahmadsamir into branch 'Plasma/5.18'.

kcm_fonts: Make the font selection dialog select the correct "Regular"-like style

Due to KConfig dropping QFont styleName property (for "Regular"-like font
styles, see [1] for more details), the font selection dialog invoked by the
KCM could end up selecting the wrong style; this change sets the appropriate
"Regular" style on the QFont object before invoking the font selection dialog
to fix/workaround the issue.

Note that in Plasma master branch the issue is handled differently, since
we switched from QFontDialog to KFontChooserDialog (the latter has that
logic built-in).

[1] https://phabricator.kde.org/D27735

M  +56   -5    kcms/fonts/fonts.cpp
M  +1    -0    kcms/fonts/fonts.h
M  +2    -5    kcms/fonts/package/contents/ui/FontWidget.qml
M  +2    -2    kcms/fonts/package/contents/ui/main.qml

https://invent.kde.org/plasma/plasma-desktop/commit/e5e5f5ed51aadfac99bfbdf3d2db5be16a12443b