D17213: Add support for settings portal

[Jan Grulich]

jgrulich created this revision.
Herald added a project: Plasma.
Herald added a subscriber: plasma-devel.
jgrulich requested review of this revision.

REVISION SUMMARY
Sandboxed applications usually don't have access to read kdeglobals configuration. For this
reason we introduced Settings portal, to be able to obtain most necessary configuration without
allowing applications access to hosts configuration. The Settings portal implementation in
xdg-desktop-portals-kde was develop mostly for plasma-integration needs so it allows to get
colors, fonts, widget theme and some other configuration from kdeglobals.

REPOSITORY
R135 Integration for Qt applications in Plasma

BRANCH
jgrulich/portal-support

REVISION DETAIL
https://phabricator.kde.org/D17213

AFFECTED FILES
src/platformtheme/kfontsettingsdata.cpp
src/platformtheme/kfontsettingsdata.h
src/platformtheme/khintssettings.cpp
src/platformtheme/khintssettings.h

To: jgrulich
Cc: plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

[Kai Uwe Broulik]

broulik added a comment.


Good stuff!

INLINE COMMENTS

kfontsettingsdata.cpp:81
- const KConfigGroup configGroup(mKdeGlobals, fontData.ConfigGroupKey);
- QString fontInfo = configGroup.readEntry(fontData.ConfigKey, QString());
+ QString fontInfo = readConfigValue(QLatin1String(fontData.ConfigGroupKey), QLatin1String(fontData.ConfigKey));

`const`

kfontsettingsdata.cpp:131
+
+ if (group == QStringLiteral("org.kde.kdeglobals.General") && key == QStringLiteral("font")) {
+ dropFontSettingsCache();

Compare with `QLatin1String`

kfontsettingsdata.h:70
+ QString readConfigValue(const QString &group, const QString &key, const QString &defaultValue = QString());
+

`const`

khintssettings.cpp:66
+
+ while (!argument.atEnd()) {
+ QString key;

I thought Qt could de-serialize built-in types on its own?

khintssettings.cpp:79
+
+static inline bool checkUsePortalSupport()
+{

Can you put that into some shared header file maybe?

khintssettings.cpp:357
+ }
+ } else if (group == QStringLiteral("org.kde.kdeglobals.Toolbar style") && key == QStringLiteral("ToolButtonStyle")) {
+ mKdeGlobalsPortal[group][key] = value.variant().toString();

Those aren't used as dbus interface names anywhere, right? (asking because of the space)

khintssettings.cpp:442
+ // Construct a temporary KConfig file containing color setting so we can create a KColorScheme from it
+ QTemporaryFile file;
+ file.open();

Would be lovely to add a `KConfig` overload to `KColorScheme::createApplicationPalette`

REPOSITORY
R135 Integration for Qt applications in Plasma

REVISION DETAIL
https://phabricator.kde.org/D17213

To: jgrulich, #plasma
Cc: broulik, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

[Jan Grulich]

jgrulich marked 5 inline comments as done.
jgrulich added inline comments.

INLINE COMMENTS

broulik wrote in khintssettings.cpp:66
I thought Qt could de-serialize built-in types on its own?

Nope, I think it can't. You already told me this before, but from my experience I always had to do it this way.

broulik wrote in khintssettings.cpp:79
Can you put that into some shared header file maybe?

They don't seem to share any header file, should I put it into khintsettings.h for example and just include it in kfontsettings.h?

broulik wrote in khintssettings.cpp:357
Those aren't used as dbus interface names anywhere, right? (asking because of the space)

No, that's just a name of group or namespace. In kdeglobals "Toolbar style" is name of group, but I had to add some prefix to somehow identify it comes from kdeglobals, because the settings portal can be used also by Gtk apps or any other apps so it has to be clear. Gtk implementation of the settings portal use for example prefix made from GSettings scheme name.

broulik wrote in khintssettings.cpp:442
Would be lovely to add a `KConfig` overload to `KColorScheme::createApplicationPalette`

Ok, I'll look into that.

REPOSITORY
R135 Integration for Qt applications in Plasma

REVISION DETAIL
https://phabricator.kde.org/D17213

To: jgrulich, #plasma
Cc: broulik, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

[Jan Grulich]

jgrulich updated this revision to Diff 46457.
jgrulich marked 2 inline comments as done.
jgrulich added a comment.


Minor fixes

REPOSITORY
R135 Integration for Qt applications in Plasma

CHANGES SINCE LAST UPDATE
https://phabricator.kde.org/D17213?vs=46399&id=46457

BRANCH
jgrulich/portal-support

REVISION DETAIL
https://phabricator.kde.org/D17213

AFFECTED FILES
src/platformtheme/kfontsettingsdata.cpp
src/platformtheme/kfontsettingsdata.h
src/platformtheme/khintssettings.cpp
src/platformtheme/khintssettings.h

To: jgrulich, #plasma
Cc: broulik, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

[Jan Grulich]

jgrulich added inline comments.

INLINE COMMENTS

jgrulich wrote in khintssettings.cpp:442
Ok, I'll look into that.

It looks it's not that simple, this change would require other additions in order to support KConfig overload. I would leave it as it is for now.

REPOSITORY
R135 Integration for Qt applications in Plasma

REVISION DETAIL
https://phabricator.kde.org/D17213

To: jgrulich, #plasma
Cc: broulik, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

[Jan Grulich]

jgrulich added a comment.


@broulik, may I ask you for review?

REPOSITORY
R135 Integration for Qt applications in Plasma

REVISION DETAIL
https://phabricator.kde.org/D17213

To: jgrulich, #plasma
Cc: broulik, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

[Jan Grulich]

This revision was automatically updated to reflect the committed changes.
Closed by commit R135:64002af69a86: Add support for settings portal (authored by jgrulich).

REPOSITORY
R135 Integration for Qt applications in Plasma

CHANGES SINCE LAST UPDATE
https://phabricator.kde.org/D17213?vs=46457&id=46831

REVISION DETAIL
https://phabricator.kde.org/D17213

AFFECTED FILES
src/platformtheme/kfontsettingsdata.cpp
src/platformtheme/kfontsettingsdata.h
src/platformtheme/khintssettings.cpp
src/platformtheme/khintssettings.h

To: jgrulich, #plasma, broulik
Cc: broulik, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart