Re: Review Request 126198: [OS X] adaptations for the KdePlatformTheme (and autotests)

[René J . V . Bertin]

> On Dec. 4, 2015, 8:26 a.m., Martin Gräßlin wrote:
> > Please include Hugo for a review on the KStyle changes.
> > 
> > I'd suggest to split the review into three parts: one about the adjusted 
> > test (ecm_foo) - that's a no brainer and doesn't need further discussion. 
> > One about the KStyle change and one about the platform theme change. These 
> > are independent libraries so a split review makes it easier IMHO.

split-offs created; I found only 1 Hugo in the list of known people, whom I 
included on the new KStyle RR.


> On Dec. 4, 2015, 8:26 a.m., Martin Gräßlin wrote:
> > src/platformtheme/CMakeLists.txt, lines 67-69
> > 
> >
> > don't make thie an elsif, just an if. It's not an elseif condition

In practice it is, I think it has more or less been decided to assume `APPLE` 
means "no X11". Qt's XCB QPA can currently still be built on OS X (and is 
almost completely functional) but there's no guarantee how long that will still 
be possible.

Which should suit some of you, as full-blown X11 support would definitely give 
sense to supporting full-blown Plasma sessions (in so-called rooted mode of the 
XQuartz server). :P


> On Dec. 4, 2015, 8:26 a.m., Martin Gräßlin wrote:
> > src/platformtheme/kdeplatformtheme.h, lines 41-54
> > 
> >
> > why did you add the virtual? The methods are marked as Q_DECL_OVERRIDE 
> > which implies they are virtual

Q_DECL_OVERRIDE indeed appears to resolve to `override` even with clang; what 
about when it doesn't? Not supposed to happen and thus not to worry about any 
issues that might cause?

For now I've left the virtual keywords on definitions that lack 
Q_DECL_OVERRIDE; should I use Q_DECL_OVERRIDE instead?


> On Dec. 4, 2015, 8:26 a.m., Martin Gräßlin wrote:
> > src/platformtheme/kdeplatformtheme.h, lines 56-61
> > 
> >
> > there is no such thing as a protected variable (see e.g. 
> > https://programmers.stackexchange.com/questions/162643/why-is-clean-code-suggesting-avoiding-protected-variables
> >  ). If you need to access the variables, please add protected accessor and 
> > setter methods

That article suggests protected member variables do exist but should be 
avoided. Is that what you meant?

The only one I actually used was mKdeGlobals from the fontsettings class


> On Dec. 4, 2015, 8:26 a.m., Martin Gräßlin wrote:
> > src/platformtheme/main_mac.cpp, line 45
> > 
> >
> > why does the mac platform want to setupXcbFlush?

If it won't ever be signalled "from outside" then no, there's no need to 
provide the slot at all. (There was a question about this on line 44 ;))

I take it the moc include is still required because the class derives QObject?


- René J.V.


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/126198/#review89105
---


On Dec. 3, 2015, 10:51 p.m., René J.V. Bertin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126198/
> ---
> 
> (Updated Dec. 3, 2015, 10:51 p.m.)
> 
> 
> Review request for KDE Software on Mac OS X, KDE Frameworks and Valorie 
> Zimmerman.
> 
> 
> Repository: frameworkintegration
> 
> 
> Description
> ---
> 
> The KdePlatformTheme plugin can be enabled on OS X with a minimal patch to 
> Qt5; all that is required is to include the `qgenericunixservices` and 
> `qgenericunixthemes` components in the build, and to append `"kde"` to the 
> list returned by `QCocoaIntegration::themeNames()` for instance under the 
> condition that `KDE_SESSION_VERSION` is set to a suitable value in the 
> environment.
> 
> This will allow KF5 and Qt5 applications to use the theme selected through 
> KDE if FrameworkIntegration is installed and KDE_SESSION_VERSION is set, 
> which seems like a sufficiently specific set of conditions that is easy to 
> avoid by users who prefer to use the Mac native theme.
> 
> While requestion the KDE theme is also possible through `-style kde` or `env 
> QT_STYLE_OVERRIDE=kde` the use of the KdePlatformTheme plugin appears to be 
> the only way to get the full theme, including the font and colour selection. 
> In my opinion it is above all the font customisation which is a very welcome 
> feature for Qt/Mac; by default Qt applications use the default system font 
> (Lucida Grande 13pt or even 14pt) throughout. This is a good UI font, but not 
> at that size (and most "native" OS X applications indeed use a range of 
> smaller sizes, depending on 

Re: Review Request 126198: [OS X] adaptations for the KdePlatformTheme (and autotests)

[Martin Gräßlin]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/126198/#review89105
---


Please include Hugo for a review on the KStyle changes.

I'd suggest to split the review into three parts: one about the adjusted test 
(ecm_foo) - that's a no brainer and doesn't need further discussion. One about 
the KStyle change and one about the platform theme change. These are 
independent libraries so a split review makes it easier IMHO.


src/platformtheme/CMakeLists.txt (lines 67 - 69)


don't make thie an elsif, just an if. It's not an elseif condition



src/platformtheme/kdeplatformtheme.h (lines 40 - 53)


why did you add the virtual? The methods are marked as Q_DECL_OVERRIDE 
which implies they are virtual



src/platformtheme/kdeplatformtheme.h (lines 55 - 60)


there is no such thing as a protected variable (see e.g. 
https://programmers.stackexchange.com/questions/162643/why-is-clean-code-suggesting-avoiding-protected-variables
 ). If you need to access the variables, please add protected accessor and 
setter methods



src/platformtheme/main_mac.cpp (line 45)


why does the mac platform want to setupXcbFlush?


- Martin Gräßlin


On Dec. 3, 2015, 10:51 p.m., René J.V. Bertin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126198/
> ---
> 
> (Updated Dec. 3, 2015, 10:51 p.m.)
> 
> 
> Review request for KDE Software on Mac OS X, KDE Frameworks and Valorie 
> Zimmerman.
> 
> 
> Repository: frameworkintegration
> 
> 
> Description
> ---
> 
> The KdePlatformTheme plugin can be enabled on OS X with a minimal patch to 
> Qt5; all that is required is to include the `qgenericunixservices` and 
> `qgenericunixthemes` components in the build, and to append `"kde"` to the 
> list returned by `QCocoaIntegration::themeNames()` for instance under the 
> condition that `KDE_SESSION_VERSION` is set to a suitable value in the 
> environment.
> 
> This will allow KF5 and Qt5 applications to use the theme selected through 
> KDE if FrameworkIntegration is installed and KDE_SESSION_VERSION is set, 
> which seems like a sufficiently specific set of conditions that is easy to 
> avoid by users who prefer to use the Mac native theme.
> 
> While requestion the KDE theme is also possible through `-style kde` or `env 
> QT_STYLE_OVERRIDE=kde` the use of the KdePlatformTheme plugin appears to be 
> the only way to get the full theme, including the font and colour selection. 
> In my opinion it is above all the font customisation which is a very welcome 
> feature for Qt/Mac; by default Qt applications use the default system font 
> (Lucida Grande 13pt or even 14pt) throughout. This is a good UI font, but not 
> at that size (and most "native" OS X applications indeed use a range of 
> smaller sizes, depending on role.
> 
> It does have introduce a number of regressions, which the current patch aims 
> to address. The most visible and problematic of these regressions is the loss 
> of the Mac-style menu bar and thus of all menu items (actions).
> 
> The fix is straightforward : on OS X (and similarly affected platforms?), an 
> instance of the native Cocoa platform theme is created through the private 
> API, and used as a fallback rather than immediately falling back to the 
> default implementations from `QPlatformTheme`. In addition, methods missing 
> from (not overridden by) `KdePlatformTheme` are provided on OS X and call the 
> corresponding methods from the native theme. It is this change which restores 
> the menubar and even the Dock menu functionality.
> One minor regression remains but should be easy to fix (elsewhere?): the 
> Preferences menu loses its keyboard shortcut (Command-,).
> 
> Given the fallback nature of the native platform instance I have preferred to 
> print a warning rather than using something like `qFatal`, above all because 
> the message printed by qFatal tends to get lost on OS X. I can replace my use 
> of `qWarning` with a dialog giving the choice between continuing or exiting 
> the application - code that would be called in the menu methods because only 
> there is it certain that the application actually needs a menubar.
> 
> In line with experience and feedback from the KDE(4)-Mac community I have 
> decided to force the use of native dialogs rather than the ones from the 
> KdePlatformPlugin.
> 
> In addition I set the fallback value for `ShowIconsOnPushButtons` to false in 
> line with platform guidelines, and ensure that the 

Re: Review Request 126198: [OS X] adaptations for the KdePlatformTheme (and autotests)

[René J . V . Bertin]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/126198/
---

(Updated Dec. 3, 2015, 10:51 p.m.)


Review request for KDE Software on Mac OS X, KDE Frameworks and Valorie 
Zimmerman.


Changes
---

A slightly polished version. New here is support for the fact that OS X will 
use a bold `MessageBoxFont` by default (bold version of the system 
font/`GeneralFont`). This is now supported through a new cache entry 
corresponding to the bold `GeneralFont`. If however the user configured a 
different `GeneralFont`, this choice will be respected, but this font will 
still be made bold.

Interestingly, the `MessageBoxFont` is one of the fonts that OS X allows/ed to 
be configured (through a utility like `TinkerTool` that exposed preferences not 
otherwise exposed).


Repository: frameworkintegration


Description
---

The KdePlatformTheme plugin can be enabled on OS X with a minimal patch to Qt5; 
all that is required is to include the `qgenericunixservices` and 
`qgenericunixthemes` components in the build, and to append `"kde"` to the list 
returned by `QCocoaIntegration::themeNames()` for instance under the condition 
that `KDE_SESSION_VERSION` is set to a suitable value in the environment.

This will allow KF5 and Qt5 applications to use the theme selected through KDE 
if FrameworkIntegration is installed and KDE_SESSION_VERSION is set, which 
seems like a sufficiently specific set of conditions that is easy to avoid by 
users who prefer to use the Mac native theme.

While requestion the KDE theme is also possible through `-style kde` or `env 
QT_STYLE_OVERRIDE=kde` the use of the KdePlatformTheme plugin appears to be the 
only way to get the full theme, including the font and colour selection. In my 
opinion it is above all the font customisation which is a very welcome feature 
for Qt/Mac; by default Qt applications use the default system font (Lucida 
Grande 13pt or even 14pt) throughout. This is a good UI font, but not at that 
size (and most "native" OS X applications indeed use a range of smaller sizes, 
depending on role.

It does have introduce a number of regressions, which the current patch aims to 
address. The most visible and problematic of these regressions is the loss of 
the Mac-style menu bar and thus of all menu items (actions).

The fix is straightforward : on OS X (and similarly affected platforms?), an 
instance of the native Cocoa platform theme is created through the private API, 
and used as a fallback rather than immediately falling back to the default 
implementations from `QPlatformTheme`. In addition, methods missing from (not 
overridden by) `KdePlatformTheme` are provided on OS X and call the 
corresponding methods from the native theme. It is this change which restores 
the menubar and even the Dock menu functionality.
One minor regression remains but should be easy to fix (elsewhere?): the 
Preferences menu loses its keyboard shortcut (Command-,).

Given the fallback nature of the native platform instance I have preferred to 
print a warning rather than using something like `qFatal`, above all because 
the message printed by qFatal tends to get lost on OS X. I can replace my use 
of `qWarning` with a dialog giving the choice between continuing or exiting the 
application - code that would be called in the menu methods because only there 
is it certain that the application actually needs a menubar.

In line with experience and feedback from the KDE(4)-Mac community I have 
decided to force the use of native dialogs rather than the ones from the 
KdePlatformPlugin.

In addition I set the fallback value for `ShowIconsOnPushButtons` to false in 
line with platform guidelines, and ensure that the autotests are not built as 
app bundles.


Diffs (updated)
-

  autotests/CMakeLists.txt 7c2129c 
  src/kstyle/CMakeLists.txt bc26667 
  src/kstyle/kstyle.mm PRE-CREATION 
  src/platformtheme/CMakeLists.txt 23f590e 
  src/platformtheme/kdemactheme.h PRE-CREATION 
  src/platformtheme/kdemactheme.mm PRE-CREATION 
  src/platformtheme/kdeplatformtheme.h 97d09df 
  src/platformtheme/kdeplatformtheme.cpp 80dbcb7 
  src/platformtheme/kfontsettingsdata.h 4b92c7d 
  src/platformtheme/kfontsettingsdatamac.h PRE-CREATION 
  src/platformtheme/kfontsettingsdatamac.mm PRE-CREATION 
  src/platformtheme/khintssettings.h ec064d3 
  src/platformtheme/khintssettingsmac.h PRE-CREATION 
  src/platformtheme/khintssettingsmac.mm PRE-CREATION 
  src/platformtheme/main_mac.cpp PRE-CREATION 

Diff: https://git.reviewboard.kde.org/r/126198/diff/


Testing
---

On Mac OS X with Qt 5.5.1, KF5 frameworks 5.16.0 and QtCurve git/head.

I have not verified to what extent my use of a private `QGuiApplication` API 
links builds to a specific Qt version (I consider that nothing shocking and a 
minor price to pay).
>>> Do I need to add some glue to the 

Re: Review Request 126198: [OS X] adaptations for the KdePlatformTheme (and autotests)

[René J . V . Bertin]

> On Dec. 2, 2015, 10:45 a.m., Boudhayan Gupta wrote:
> > src/platformtheme/kdemactheme.mm, line 39
> > 
> >
> > This makes me very nervous.
> > 
> > Using private APIs is almost always a guarantee the application won't 
> > preserve binary compatibility even across point releases (eg Qt 5.5.0 - 
> > 5.5.1), let alone across major releases.
> > 
> > What makes it even more dangerous is that this file won't be built only 
> > on MacPorts/Fink but also by people making normal AppBundle releases - 
> > where you have no control over what versions of dependencies are being used.
> > 
> > Please try to find a public API alternative, even if it ends up being a 
> > giant hack. I once found a very elegant private API solution to making a 
> > QQuickWidget emit a release event on the mouse cursor, but just because of 
> > the whole binary compat problem (some Linux distros don't even ship apps 
> > depending on private headers) I had to create a dummy item inside the 
> > QtQuick code and interact with it to get the cursor released. Giant hack, 
> > but at least no private API usage.
> > 
> > Private API is **private** for a reason.
> 
> Luigi Toscano wrote:
> Afaik frameworkintegration is supposed to be an extension of the internal 
> Qt world and it has already been the case (I asked the same question few 
> Akademys ago). If you want to use QPA, you need to go down in the stack.
> 
> Martin Gräßlin wrote:
> yeah frameworkintegration is bound to use private API. So not really a 
> problem in this special case.
> 
> AFAIK some distros have adjusted the packaging to force a recompile for 
> new Qt releases.
> 
> Boudhayan Gupta wrote:
> Ah, that makes sense. Okay, I'm dropping this issue.
> 
> René J.V. Bertin wrote:
> Whew - Hacking is so much more fun if you can do it there where you're 
> not supposed to go, rather than to avoid exactly that ;)
> However, it might be possible to replace the direct call of the private 
> function by a call through a function pointer to `createPlatformTheme` 
> obtained through a dynamic resolver. Any idea if that is bound to fail 
> through a Qt function because it'd detect a request for a private API?
> 
> In any case, you (Boudhayan) are right that this modified framework might 
> end up being used also in *standalone* AppBundle builds rather than only in 
> builds using shared resources (in which applications are also built as app 
> bundles!). At least I hope it will, for the very reason I've been pushing the 
> patch. But I think you're wrong to assume that using private APIs is a 
> problem there. If anything it should be (much) less the case, because 
> standalone app bundles form a much more protected environment. They're not 
> meant to be upgraded internally, except maybe by some special mechanism 
> controlled by the developer(s) in charge. Updates to Qt without an 
> accompanying forced rebuild ("revbump" in MacPorts speak) to 
> frameworkintegration are much more likely to occur in a shared environment.
> 
> Boudhayan Gupta wrote:
> There's no reason to go through a dynamic function resolver if the rest 
> of FWI uses private APIs happily. Luigi says, "frameworkintegration is 
> supposed to be an extension of the internal Qt world" - in that case it makes 
> perfect sense to make use of private APIs to get your job done.
> 
> As for the AppBundles point - I'm thinking if it's possible Qt is 
> installed from e.g. the SDKs provided by qt.io and standalone AppBundles end 
> up depending on that. If not, using private APIs in standalone AppBundles 
> makes no difference - you won't upgrade Qt without upgrading the entire 
> AppBundle.

Dynamic resolution appears out of the question anyway, because the 
createPlatformTheme function is not a static member function. There's a `static 
QCocoaIntegration *instance()` (`_ZN17QCocoaIntegration8instanceEv`), though, 
which should return `QGuiApplicationPrivate::platform_integration`. Regardless 
of whether it's acceptable to use a private API here, there is still the 
question of handling a conflict/mismatch situation in a sensible way, one in 
which it's apparent immediately what went wrong.
But maybe there's an easier way to do that, simply by comparing a hard-coded Qt 
version with the runtime version.

Standalone app bundles are by definition not dependent on anything but system 
libraries, that's the whole idea behind them. When signed (e.g. because shipped 
through the App Store) you can also not modify them because if you do they 
won't launch. Of course you can build partially standalone app bundles, but I 
think that most devs will either aim for truly standalone products, or else 
product that share as much as possible (in which case Qt + KF5 frameworks could 
be built as a composite framework bundle).


- René J.V.

Re: Review Request 126198: [OS X] adaptations for the KdePlatformTheme (and autotests)

[René J . V . Bertin]

> On Dec. 2, 2015, 10:45 a.m., Boudhayan Gupta wrote:
> > src/platformtheme/kdemactheme.mm, line 39
> > 
> >
> > This makes me very nervous.
> > 
> > Using private APIs is almost always a guarantee the application won't 
> > preserve binary compatibility even across point releases (eg Qt 5.5.0 - 
> > 5.5.1), let alone across major releases.
> > 
> > What makes it even more dangerous is that this file won't be built only 
> > on MacPorts/Fink but also by people making normal AppBundle releases - 
> > where you have no control over what versions of dependencies are being used.
> > 
> > Please try to find a public API alternative, even if it ends up being a 
> > giant hack. I once found a very elegant private API solution to making a 
> > QQuickWidget emit a release event on the mouse cursor, but just because of 
> > the whole binary compat problem (some Linux distros don't even ship apps 
> > depending on private headers) I had to create a dummy item inside the 
> > QtQuick code and interact with it to get the cursor released. Giant hack, 
> > but at least no private API usage.
> > 
> > Private API is **private** for a reason.
> 
> Luigi Toscano wrote:
> Afaik frameworkintegration is supposed to be an extension of the internal 
> Qt world and it has already been the case (I asked the same question few 
> Akademys ago). If you want to use QPA, you need to go down in the stack.
> 
> Martin Gräßlin wrote:
> yeah frameworkintegration is bound to use private API. So not really a 
> problem in this special case.
> 
> AFAIK some distros have adjusted the packaging to force a recompile for 
> new Qt releases.
> 
> Boudhayan Gupta wrote:
> Ah, that makes sense. Okay, I'm dropping this issue.

Whew - Hacking is so much more fun if you can do it there where you're not 
supposed to go, rather than to avoid exactly that ;)
However, it might be possible to replace the direct call of the private 
function by a call through a function pointer to `createPlatformTheme` obtained 
through a dynamic resolver. Any idea if that is bound to fail through a Qt 
function because it'd detect a request for a private API?

In any case, you (Boudhayan) are right that this modified framework might end 
up being used also in *standalone* AppBundle builds rather than only in builds 
using shared resources (in which applications are also built as app bundles!). 
At least I hope it will, for the very reason I've been pushing the patch. But I 
think you're wrong to assume that using private APIs is a problem there. If 
anything it should be (much) less the case, because standalone app bundles form 
a much more protected environment. They're not meant to be upgraded internally, 
except maybe by some special mechanism controlled by the developer(s) in 
charge. Updates to Qt without an accompanying forced rebuild ("revbump" in 
MacPorts speak) to frameworkintegration are much more likely to occur in a 
shared environment.


- René J.V.


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/126198/#review89026
---


On Dec. 1, 2015, 9:29 p.m., René J.V. Bertin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126198/
> ---
> 
> (Updated Dec. 1, 2015, 9:29 p.m.)
> 
> 
> Review request for KDE Software on Mac OS X, KDE Frameworks and Valorie 
> Zimmerman.
> 
> 
> Repository: frameworkintegration
> 
> 
> Description
> ---
> 
> The KdePlatformTheme plugin can be enabled on OS X with a minimal patch to 
> Qt5; all that is required is to include the `qgenericunixservices` and 
> `qgenericunixthemes` components in the build, and to append `"kde"` to the 
> list returned by `QCocoaIntegration::themeNames()` for instance under the 
> condition that `KDE_SESSION_VERSION` is set to a suitable value in the 
> environment.
> 
> This will allow KF5 and Qt5 applications to use the theme selected through 
> KDE if FrameworkIntegration is installed and KDE_SESSION_VERSION is set, 
> which seems like a sufficiently specific set of conditions that is easy to 
> avoid by users who prefer to use the Mac native theme.
> 
> While requestion the KDE theme is also possible through `-style kde` or `env 
> QT_STYLE_OVERRIDE=kde` the use of the KdePlatformTheme plugin appears to be 
> the only way to get the full theme, including the font and colour selection. 
> In my opinion it is above all the font customisation which is a very welcome 
> feature for Qt/Mac; by default Qt applications use the default system font 
> (Lucida Grande 13pt or even 14pt) throughout. This is a good UI font, but not 
> at that size 

Re: Review Request 126198: [OS X] adaptations for the KdePlatformTheme (and autotests)

[Boudhayan Gupta]

> On Dec. 2, 2015, 3:15 p.m., Boudhayan Gupta wrote:
> > src/platformtheme/kdemactheme.mm, line 39
> > 
> >
> > This makes me very nervous.
> > 
> > Using private APIs is almost always a guarantee the application won't 
> > preserve binary compatibility even across point releases (eg Qt 5.5.0 - 
> > 5.5.1), let alone across major releases.
> > 
> > What makes it even more dangerous is that this file won't be built only 
> > on MacPorts/Fink but also by people making normal AppBundle releases - 
> > where you have no control over what versions of dependencies are being used.
> > 
> > Please try to find a public API alternative, even if it ends up being a 
> > giant hack. I once found a very elegant private API solution to making a 
> > QQuickWidget emit a release event on the mouse cursor, but just because of 
> > the whole binary compat problem (some Linux distros don't even ship apps 
> > depending on private headers) I had to create a dummy item inside the 
> > QtQuick code and interact with it to get the cursor released. Giant hack, 
> > but at least no private API usage.
> > 
> > Private API is **private** for a reason.
> 
> Luigi Toscano wrote:
> Afaik frameworkintegration is supposed to be an extension of the internal 
> Qt world and it has already been the case (I asked the same question few 
> Akademys ago). If you want to use QPA, you need to go down in the stack.
> 
> Martin Gräßlin wrote:
> yeah frameworkintegration is bound to use private API. So not really a 
> problem in this special case.
> 
> AFAIK some distros have adjusted the packaging to force a recompile for 
> new Qt releases.
> 
> Boudhayan Gupta wrote:
> Ah, that makes sense. Okay, I'm dropping this issue.
> 
> René J.V. Bertin wrote:
> Whew - Hacking is so much more fun if you can do it there where you're 
> not supposed to go, rather than to avoid exactly that ;)
> However, it might be possible to replace the direct call of the private 
> function by a call through a function pointer to `createPlatformTheme` 
> obtained through a dynamic resolver. Any idea if that is bound to fail 
> through a Qt function because it'd detect a request for a private API?
> 
> In any case, you (Boudhayan) are right that this modified framework might 
> end up being used also in *standalone* AppBundle builds rather than only in 
> builds using shared resources (in which applications are also built as app 
> bundles!). At least I hope it will, for the very reason I've been pushing the 
> patch. But I think you're wrong to assume that using private APIs is a 
> problem there. If anything it should be (much) less the case, because 
> standalone app bundles form a much more protected environment. They're not 
> meant to be upgraded internally, except maybe by some special mechanism 
> controlled by the developer(s) in charge. Updates to Qt without an 
> accompanying forced rebuild ("revbump" in MacPorts speak) to 
> frameworkintegration are much more likely to occur in a shared environment.

There's no reason to go through a dynamic function resolver if the rest of FWI 
uses private APIs happily. Luigi says, "frameworkintegration is supposed to be 
an extension of the internal Qt world" - in that case it makes perfect sense to 
make use of private APIs to get your job done.

As for the AppBundles point - I'm thinking if it's possible Qt is installed 
from e.g. the SDKs provided by qt.io and standalone AppBundles end up depending 
on that. If not, using private APIs in standalone AppBundles makes no 
difference - you won't upgrade Qt without upgrading the entire AppBundle.


- Boudhayan


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/126198/#review89026
---


On Dec. 2, 2015, 1:59 a.m., René J.V. Bertin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126198/
> ---
> 
> (Updated Dec. 2, 2015, 1:59 a.m.)
> 
> 
> Review request for KDE Software on Mac OS X, KDE Frameworks and Valorie 
> Zimmerman.
> 
> 
> Repository: frameworkintegration
> 
> 
> Description
> ---
> 
> The KdePlatformTheme plugin can be enabled on OS X with a minimal patch to 
> Qt5; all that is required is to include the `qgenericunixservices` and 
> `qgenericunixthemes` components in the build, and to append `"kde"` to the 
> list returned by `QCocoaIntegration::themeNames()` for instance under the 
> condition that `KDE_SESSION_VERSION` is set to a suitable value in the 
> environment.
> 
> This will allow KF5 and Qt5 applications to use the theme selected through 
> KDE if FrameworkIntegration is 

Re: Review Request 126198: [OS X] adaptations for the KdePlatformTheme (and autotests)

[René J . V . Bertin]

> On Dec. 1, 2015, 9:34 p.m., René J.V. Bertin wrote:
> > src/platformtheme/kdemactheme.mm, lines 53-87
> > 
> >
> > I thought it would be best to use a native dialog here to show a 
> > warning dialog, but it turns out there is already a native event filter in 
> > place that causes a conflict and a crash.
> > I'll have to replace this with a QMessageBox.

QMessageBox isn't possible either because we don't have a QApplication instance 
at this point.
It *should* be possible to use a native dialog as shown above, but then I'd 
need to find a way to deactivate the nativeEventFilter, and reactivate it after 
the dialog has been closed.


- René J.V.


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/126198/#review89009
---


On Dec. 1, 2015, 9:29 p.m., René J.V. Bertin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126198/
> ---
> 
> (Updated Dec. 1, 2015, 9:29 p.m.)
> 
> 
> Review request for KDE Software on Mac OS X, KDE Frameworks and Valorie 
> Zimmerman.
> 
> 
> Repository: frameworkintegration
> 
> 
> Description
> ---
> 
> The KdePlatformTheme plugin can be enabled on OS X with a minimal patch to 
> Qt5; all that is required is to include the `qgenericunixservices` and 
> `qgenericunixthemes` components in the build, and to append `"kde"` to the 
> list returned by `QCocoaIntegration::themeNames()` for instance under the 
> condition that `KDE_SESSION_VERSION` is set to a suitable value in the 
> environment.
> 
> This will allow KF5 and Qt5 applications to use the theme selected through 
> KDE if FrameworkIntegration is installed and KDE_SESSION_VERSION is set, 
> which seems like a sufficiently specific set of conditions that is easy to 
> avoid by users who prefer to use the Mac native theme.
> 
> While requestion the KDE theme is also possible through `-style kde` or `env 
> QT_STYLE_OVERRIDE=kde` the use of the KdePlatformTheme plugin appears to be 
> the only way to get the full theme, including the font and colour selection. 
> In my opinion it is above all the font customisation which is a very welcome 
> feature for Qt/Mac; by default Qt applications use the default system font 
> (Lucida Grande 13pt or even 14pt) throughout. This is a good UI font, but not 
> at that size (and most "native" OS X applications indeed use a range of 
> smaller sizes, depending on role.
> 
> It does have introduce a number of regressions, which the current patch aims 
> to address. The most visible and problematic of these regressions is the loss 
> of the Mac-style menu bar and thus of all menu items (actions).
> 
> The fix is straightforward : on OS X (and similarly affected platforms?), an 
> instance of the native Cocoa platform theme is created through the private 
> API, and used as a fallback rather than immediately falling back to the 
> default implementations from `QPlatformTheme`. In addition, methods missing 
> from (not overridden by) `KdePlatformTheme` are provided on OS X and call the 
> corresponding methods from the native theme. It is this change which restores 
> the menubar and even the Dock menu functionality.
> One minor regression remains but should be easy to fix (elsewhere?): the 
> Preferences menu loses its keyboard shortcut (Command-,).
> 
> Given the fallback nature of the native platform instance I have preferred to 
> print a warning rather than using something like `qFatal`, above all because 
> the message printed by qFatal tends to get lost on OS X. I can replace my use 
> of `qWarning` with a dialog giving the choice between continuing or exiting 
> the application - code that would be called in the menu methods because only 
> there is it certain that the application actually needs a menubar.
> 
> In line with experience and feedback from the KDE(4)-Mac community I have 
> decided to force the use of native dialogs rather than the ones from the 
> KdePlatformPlugin.
> 
> In addition I set the fallback value for `ShowIconsOnPushButtons` to false in 
> line with platform guidelines, and ensure that the autotests are not built as 
> app bundles.
> 
> 
> Diffs
> -
> 
>   autotests/CMakeLists.txt 7c2129c 
>   src/kstyle/CMakeLists.txt bc26667 
>   src/kstyle/kstyle.mm PRE-CREATION 
>   src/platformtheme/CMakeLists.txt 23f590e 
>   src/platformtheme/kdemactheme.h PRE-CREATION 
>   src/platformtheme/kdemactheme.mm PRE-CREATION 
>   src/platformtheme/khintssettingsmac.h PRE-CREATION 
>   src/platformtheme/khintssettingsmac.mm PRE-CREATION 
>   src/platformtheme/main_mac.cpp PRE-CREATION 
> 
> Diff: https://git.reviewboard.kde.org/r/126198/diff/
> 
> 
> 

Re: Review Request 126198: [OS X] adaptations for the KdePlatformTheme (and autotests)

[René J . V . Bertin]

> On Dec. 2, 2015, 8:46 a.m., Martin Gräßlin wrote:
> > Overall I think this is now too much code duplication. With this appraoch 
> > you don't get bug fixes from the base code. I recommed to rather go for 
> > inheritance to have the actual code which can be shared still together.

Yes, I was aware of that; it was part of what I had in mind when I mentioned 
the maintenance burden being on KDE-Mac. 
I'll see what kind of difference inheritance can make here. I've been going 
over khintsettingsmac.mm because of a crash in there when testing without a 
kdeglobals file present. I'd left some look-and-feel remnants, and I think 
`KHintsSettings::readConfigValue()` will probably want to check if 
`mDefaultLnfConfig` isn't NULL before using it. 
Anyway, my impression is that even inheritance this class will be overriding 
(and thus duplicating) the brunt of the code; should I try it anyway?


> On Dec. 2, 2015, 8:46 a.m., Martin Gräßlin wrote:
> > src/platformtheme/CMakeLists.txt, lines 22-46
> > 
> >
> > please change this part to only contain differences. Otherwise it 
> > becomes difficult to maintain.
> > 
> > Like
> > 
> > set(platformtheme_SRCS ...)
> > if (APPLE)
> > set(platformtheme_SRCS ${platformtheme_SRCS} foo.mm)
> > endif()
> > 
> > 
> > Ideally I also don't want the generic part in the else branch. This 
> > should be more a feature based approach. What I don't want is that we 
> > generate a setup where it goes if(APPLE) else if (WINDOWS) else if 
> > (ANDROID)... You get what I mean ;-)

Erm, yes. Not sure why I didn't do this in the first place, maybe as a 
subconscious preparation for a possible need to fork and roll a related 
framework not intended to be Plasma-specific ;)


- René J.V.


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/126198/#review89021
---


On Dec. 1, 2015, 9:29 p.m., René J.V. Bertin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126198/
> ---
> 
> (Updated Dec. 1, 2015, 9:29 p.m.)
> 
> 
> Review request for KDE Software on Mac OS X, KDE Frameworks and Valorie 
> Zimmerman.
> 
> 
> Repository: frameworkintegration
> 
> 
> Description
> ---
> 
> The KdePlatformTheme plugin can be enabled on OS X with a minimal patch to 
> Qt5; all that is required is to include the `qgenericunixservices` and 
> `qgenericunixthemes` components in the build, and to append `"kde"` to the 
> list returned by `QCocoaIntegration::themeNames()` for instance under the 
> condition that `KDE_SESSION_VERSION` is set to a suitable value in the 
> environment.
> 
> This will allow KF5 and Qt5 applications to use the theme selected through 
> KDE if FrameworkIntegration is installed and KDE_SESSION_VERSION is set, 
> which seems like a sufficiently specific set of conditions that is easy to 
> avoid by users who prefer to use the Mac native theme.
> 
> While requestion the KDE theme is also possible through `-style kde` or `env 
> QT_STYLE_OVERRIDE=kde` the use of the KdePlatformTheme plugin appears to be 
> the only way to get the full theme, including the font and colour selection. 
> In my opinion it is above all the font customisation which is a very welcome 
> feature for Qt/Mac; by default Qt applications use the default system font 
> (Lucida Grande 13pt or even 14pt) throughout. This is a good UI font, but not 
> at that size (and most "native" OS X applications indeed use a range of 
> smaller sizes, depending on role.
> 
> It does have introduce a number of regressions, which the current patch aims 
> to address. The most visible and problematic of these regressions is the loss 
> of the Mac-style menu bar and thus of all menu items (actions).
> 
> The fix is straightforward : on OS X (and similarly affected platforms?), an 
> instance of the native Cocoa platform theme is created through the private 
> API, and used as a fallback rather than immediately falling back to the 
> default implementations from `QPlatformTheme`. In addition, methods missing 
> from (not overridden by) `KdePlatformTheme` are provided on OS X and call the 
> corresponding methods from the native theme. It is this change which restores 
> the menubar and even the Dock menu functionality.
> One minor regression remains but should be easy to fix (elsewhere?): the 
> Preferences menu loses its keyboard shortcut (Command-,).
> 
> Given the fallback nature of the native platform instance I have preferred to 
> print a warning rather than using something like `qFatal`, above all because 
> the message printed by qFatal tends to 

Re: Review Request 126198: [OS X] adaptations for the KdePlatformTheme (and autotests)

[Martin Gräßlin]

> On Dec. 2, 2015, 8:38 a.m., Martin Gräßlin wrote:
> > Just wondering: if your main aim is to change default settings, why don't 
> > follow what linux distros do? That is ship some default configuration 
> > interface. For inspiration check e.g. kubuntu-default-settings.
> 
> René J.V. Bertin wrote:
> If I understand you correctly, you're suggesting I write a set of default 
> configuration values that could go into a look-and-feel package (?) I 
> stripped support for those in an effort to be PC because they're expected in 
> a folder called `plasma`.
> 
> It does look like this idea could remove the need for a Mac-specific 
> KHintsSettings class.
> 
> One thing I don't understand here: precedence between the package 
> configured in kdeglobals (`LookAndFeelPackage`) which is stored in 
> `mDefaultLnfConfig` and the hardcoded `defaultLookAndFeelPackage` package. 
> The latter is stored in `mLnfConfig` if different from the user-selected 
> package, but then `readConfigValue()` gives precedence to `mLnfConfig` if 
> it's set. That looks as if the effective roles are reversed.
> 
> René J.V. Bertin wrote:
> Too fast again :-/
> 
> I see that my suggestion below to check `mDefaultLnfConfig` before 
> accessing it is moot, but shouldn't `mLnfConfig` be initialised to 0 if 
> `looknfeel == defaultLookAndFeelPackage`?

> If I understand you correctly, you're suggesting I write a set of default 
> configuration values that could go into a look-and-feel package 

kconfig files. You just install a more global config file with your default 
values. Best check with some distros on how to do that. (I'm from the opposite 
department, telling distros when they broke it :-P )


- Martin


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/126198/#review89019
---


On Dec. 1, 2015, 9:29 p.m., René J.V. Bertin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126198/
> ---
> 
> (Updated Dec. 1, 2015, 9:29 p.m.)
> 
> 
> Review request for KDE Software on Mac OS X, KDE Frameworks and Valorie 
> Zimmerman.
> 
> 
> Repository: frameworkintegration
> 
> 
> Description
> ---
> 
> The KdePlatformTheme plugin can be enabled on OS X with a minimal patch to 
> Qt5; all that is required is to include the `qgenericunixservices` and 
> `qgenericunixthemes` components in the build, and to append `"kde"` to the 
> list returned by `QCocoaIntegration::themeNames()` for instance under the 
> condition that `KDE_SESSION_VERSION` is set to a suitable value in the 
> environment.
> 
> This will allow KF5 and Qt5 applications to use the theme selected through 
> KDE if FrameworkIntegration is installed and KDE_SESSION_VERSION is set, 
> which seems like a sufficiently specific set of conditions that is easy to 
> avoid by users who prefer to use the Mac native theme.
> 
> While requestion the KDE theme is also possible through `-style kde` or `env 
> QT_STYLE_OVERRIDE=kde` the use of the KdePlatformTheme plugin appears to be 
> the only way to get the full theme, including the font and colour selection. 
> In my opinion it is above all the font customisation which is a very welcome 
> feature for Qt/Mac; by default Qt applications use the default system font 
> (Lucida Grande 13pt or even 14pt) throughout. This is a good UI font, but not 
> at that size (and most "native" OS X applications indeed use a range of 
> smaller sizes, depending on role.
> 
> It does have introduce a number of regressions, which the current patch aims 
> to address. The most visible and problematic of these regressions is the loss 
> of the Mac-style menu bar and thus of all menu items (actions).
> 
> The fix is straightforward : on OS X (and similarly affected platforms?), an 
> instance of the native Cocoa platform theme is created through the private 
> API, and used as a fallback rather than immediately falling back to the 
> default implementations from `QPlatformTheme`. In addition, methods missing 
> from (not overridden by) `KdePlatformTheme` are provided on OS X and call the 
> corresponding methods from the native theme. It is this change which restores 
> the menubar and even the Dock menu functionality.
> One minor regression remains but should be easy to fix (elsewhere?): the 
> Preferences menu loses its keyboard shortcut (Command-,).
> 
> Given the fallback nature of the native platform instance I have preferred to 
> print a warning rather than using something like `qFatal`, above all because 
> the message printed by qFatal tends to get lost on OS X. I can replace my use 
> of `qWarning` with a dialog giving the choice between continuing or exiting 
> the application - code that 

Re: Review Request 126198: [OS X] adaptations for the KdePlatformTheme (and autotests)

[René J . V . Bertin]

> On Dec. 2, 2015, 8:38 a.m., Martin Gräßlin wrote:
> > Just wondering: if your main aim is to change default settings, why don't 
> > follow what linux distros do? That is ship some default configuration 
> > interface. For inspiration check e.g. kubuntu-default-settings.
> 
> René J.V. Bertin wrote:
> If I understand you correctly, you're suggesting I write a set of default 
> configuration values that could go into a look-and-feel package (?) I 
> stripped support for those in an effort to be PC because they're expected in 
> a folder called `plasma`.
> 
> It does look like this idea could remove the need for a Mac-specific 
> KHintsSettings class.
> 
> One thing I don't understand here: precedence between the package 
> configured in kdeglobals (`LookAndFeelPackage`) which is stored in 
> `mDefaultLnfConfig` and the hardcoded `defaultLookAndFeelPackage` package. 
> The latter is stored in `mLnfConfig` if different from the user-selected 
> package, but then `readConfigValue()` gives precedence to `mLnfConfig` if 
> it's set. That looks as if the effective roles are reversed.

Too fast again :-/

I see that my suggestion below to check `mDefaultLnfConfig` before accessing it 
is moot, but shouldn't `mLnfConfig` be initialised to 0 if `looknfeel == 
defaultLookAndFeelPackage`?


- René J.V.


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/126198/#review89019
---


On Dec. 1, 2015, 9:29 p.m., René J.V. Bertin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126198/
> ---
> 
> (Updated Dec. 1, 2015, 9:29 p.m.)
> 
> 
> Review request for KDE Software on Mac OS X, KDE Frameworks and Valorie 
> Zimmerman.
> 
> 
> Repository: frameworkintegration
> 
> 
> Description
> ---
> 
> The KdePlatformTheme plugin can be enabled on OS X with a minimal patch to 
> Qt5; all that is required is to include the `qgenericunixservices` and 
> `qgenericunixthemes` components in the build, and to append `"kde"` to the 
> list returned by `QCocoaIntegration::themeNames()` for instance under the 
> condition that `KDE_SESSION_VERSION` is set to a suitable value in the 
> environment.
> 
> This will allow KF5 and Qt5 applications to use the theme selected through 
> KDE if FrameworkIntegration is installed and KDE_SESSION_VERSION is set, 
> which seems like a sufficiently specific set of conditions that is easy to 
> avoid by users who prefer to use the Mac native theme.
> 
> While requestion the KDE theme is also possible through `-style kde` or `env 
> QT_STYLE_OVERRIDE=kde` the use of the KdePlatformTheme plugin appears to be 
> the only way to get the full theme, including the font and colour selection. 
> In my opinion it is above all the font customisation which is a very welcome 
> feature for Qt/Mac; by default Qt applications use the default system font 
> (Lucida Grande 13pt or even 14pt) throughout. This is a good UI font, but not 
> at that size (and most "native" OS X applications indeed use a range of 
> smaller sizes, depending on role.
> 
> It does have introduce a number of regressions, which the current patch aims 
> to address. The most visible and problematic of these regressions is the loss 
> of the Mac-style menu bar and thus of all menu items (actions).
> 
> The fix is straightforward : on OS X (and similarly affected platforms?), an 
> instance of the native Cocoa platform theme is created through the private 
> API, and used as a fallback rather than immediately falling back to the 
> default implementations from `QPlatformTheme`. In addition, methods missing 
> from (not overridden by) `KdePlatformTheme` are provided on OS X and call the 
> corresponding methods from the native theme. It is this change which restores 
> the menubar and even the Dock menu functionality.
> One minor regression remains but should be easy to fix (elsewhere?): the 
> Preferences menu loses its keyboard shortcut (Command-,).
> 
> Given the fallback nature of the native platform instance I have preferred to 
> print a warning rather than using something like `qFatal`, above all because 
> the message printed by qFatal tends to get lost on OS X. I can replace my use 
> of `qWarning` with a dialog giving the choice between continuing or exiting 
> the application - code that would be called in the menu methods because only 
> there is it certain that the application actually needs a menubar.
> 
> In line with experience and feedback from the KDE(4)-Mac community I have 
> decided to force the use of native dialogs rather than the ones from the 
> KdePlatformPlugin.
> 
> In addition I set the fallback value for `ShowIconsOnPushButtons` to false in 
> line with platform 

Re: Review Request 126198: [OS X] adaptations for the KdePlatformTheme (and autotests)

[René J . V . Bertin]

> On Dec. 2, 2015, 8:38 a.m., Martin Gräßlin wrote:
> > Just wondering: if your main aim is to change default settings, why don't 
> > follow what linux distros do? That is ship some default configuration 
> > interface. For inspiration check e.g. kubuntu-default-settings.

If I understand you correctly, you're suggesting I write a set of default 
configuration values that could go into a look-and-feel package (?) I stripped 
support for those in an effort to be PC because they're expected in a folder 
called `plasma`.

It does look like this idea could remove the need for a Mac-specific 
KHintsSettings class.

One thing I don't understand here: precedence between the package configured in 
kdeglobals (`LookAndFeelPackage`) which is stored in `mDefaultLnfConfig` and 
the hardcoded `defaultLookAndFeelPackage` package. The latter is stored in 
`mLnfConfig` if different from the user-selected package, but then 
`readConfigValue()` gives precedence to `mLnfConfig` if it's set. That looks as 
if the effective roles are reversed.


- René J.V.


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/126198/#review89019
---


On Dec. 1, 2015, 9:29 p.m., René J.V. Bertin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126198/
> ---
> 
> (Updated Dec. 1, 2015, 9:29 p.m.)
> 
> 
> Review request for KDE Software on Mac OS X, KDE Frameworks and Valorie 
> Zimmerman.
> 
> 
> Repository: frameworkintegration
> 
> 
> Description
> ---
> 
> The KdePlatformTheme plugin can be enabled on OS X with a minimal patch to 
> Qt5; all that is required is to include the `qgenericunixservices` and 
> `qgenericunixthemes` components in the build, and to append `"kde"` to the 
> list returned by `QCocoaIntegration::themeNames()` for instance under the 
> condition that `KDE_SESSION_VERSION` is set to a suitable value in the 
> environment.
> 
> This will allow KF5 and Qt5 applications to use the theme selected through 
> KDE if FrameworkIntegration is installed and KDE_SESSION_VERSION is set, 
> which seems like a sufficiently specific set of conditions that is easy to 
> avoid by users who prefer to use the Mac native theme.
> 
> While requestion the KDE theme is also possible through `-style kde` or `env 
> QT_STYLE_OVERRIDE=kde` the use of the KdePlatformTheme plugin appears to be 
> the only way to get the full theme, including the font and colour selection. 
> In my opinion it is above all the font customisation which is a very welcome 
> feature for Qt/Mac; by default Qt applications use the default system font 
> (Lucida Grande 13pt or even 14pt) throughout. This is a good UI font, but not 
> at that size (and most "native" OS X applications indeed use a range of 
> smaller sizes, depending on role.
> 
> It does have introduce a number of regressions, which the current patch aims 
> to address. The most visible and problematic of these regressions is the loss 
> of the Mac-style menu bar and thus of all menu items (actions).
> 
> The fix is straightforward : on OS X (and similarly affected platforms?), an 
> instance of the native Cocoa platform theme is created through the private 
> API, and used as a fallback rather than immediately falling back to the 
> default implementations from `QPlatformTheme`. In addition, methods missing 
> from (not overridden by) `KdePlatformTheme` are provided on OS X and call the 
> corresponding methods from the native theme. It is this change which restores 
> the menubar and even the Dock menu functionality.
> One minor regression remains but should be easy to fix (elsewhere?): the 
> Preferences menu loses its keyboard shortcut (Command-,).
> 
> Given the fallback nature of the native platform instance I have preferred to 
> print a warning rather than using something like `qFatal`, above all because 
> the message printed by qFatal tends to get lost on OS X. I can replace my use 
> of `qWarning` with a dialog giving the choice between continuing or exiting 
> the application - code that would be called in the menu methods because only 
> there is it certain that the application actually needs a menubar.
> 
> In line with experience and feedback from the KDE(4)-Mac community I have 
> decided to force the use of native dialogs rather than the ones from the 
> KdePlatformPlugin.
> 
> In addition I set the fallback value for `ShowIconsOnPushButtons` to false in 
> line with platform guidelines, and ensure that the autotests are not built as 
> app bundles.
> 
> 
> Diffs
> -
> 
>   autotests/CMakeLists.txt 7c2129c 
>   src/kstyle/CMakeLists.txt bc26667 
>   src/kstyle/kstyle.mm PRE-CREATION 
>   src/platformtheme/CMakeLists.txt 23f590e 
>   

Re: Review Request 126198: [OS X] adaptations for the KdePlatformTheme (and autotests)

[René J . V . Bertin]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/126198/
---

(Updated Dec. 2, 2015, 10:38 p.m.)


Review request for KDE Software on Mac OS X, KDE Frameworks and Valorie 
Zimmerman.


Changes
---

This revision is a probably heavy-handed attempt at implementing Martin's 
suggestion to re-use as much code as possible using inheritance. There's less 
code that can be reused than I'd have liked, and I have a strong impression 
that my implementation actually does quite a few things twice. Meaning once in 
the parent class and once in my own class, despite probably overly liberal use 
of `virtual` keywords. Then again it seems hard to avoid this given that the 
parent KdePlatformTheme class has KHintsSettings and KFontsSettings instances, 
and my KdeMacTheme has KHintsSettingsMac and KFontsSettingsMac instances.
Clear this isn't yet my strongest subject yet, so I hope I haven't made a 
complete mess of this :-/

I've made the private members protected, in order to be able to inherit them, 
and I've introduce a `KdePlatformTheme::fontType()` method so that it becomes 
possible to have a central `QPlatformTheme::Font -> 
KFontSettingsData::FontType` translation, and then do a platform-specific 
lookup in the dedicated `m_fontsData` array. Maybe that `fontType()` method 
should be in `KFontDataSettings`?

New in this revision is also `KFontDataSettingsMac`, which exists in order to 
obtain the default fonts from the system (though the sizes are hardcoded, 
currently). The current algorithm isn't very satisfactory, as it turns out to 
be a more or less known Qt issue that it doesn't provide this information 
reliably on OS X. Surprise, surprise :) I'll have to look if this information 
can be obtained through Cocoa.

The reason to do this dynamically (and leave the default colour palette to 
whatever Qt sets it to, for now) instead of using a predefined config file is 
that this information can change between OS versions. OS X 10.11 introduced a 
new system font (or 2), and the default colours aren't set in stone either (and 
users can chose the `Graphite` or default "Aqua" theme, as well as the more 
recent "Dark mode").


Repository: frameworkintegration


Description
---

The KdePlatformTheme plugin can be enabled on OS X with a minimal patch to Qt5; 
all that is required is to include the `qgenericunixservices` and 
`qgenericunixthemes` components in the build, and to append `"kde"` to the list 
returned by `QCocoaIntegration::themeNames()` for instance under the condition 
that `KDE_SESSION_VERSION` is set to a suitable value in the environment.

This will allow KF5 and Qt5 applications to use the theme selected through KDE 
if FrameworkIntegration is installed and KDE_SESSION_VERSION is set, which 
seems like a sufficiently specific set of conditions that is easy to avoid by 
users who prefer to use the Mac native theme.

While requestion the KDE theme is also possible through `-style kde` or `env 
QT_STYLE_OVERRIDE=kde` the use of the KdePlatformTheme plugin appears to be the 
only way to get the full theme, including the font and colour selection. In my 
opinion it is above all the font customisation which is a very welcome feature 
for Qt/Mac; by default Qt applications use the default system font (Lucida 
Grande 13pt or even 14pt) throughout. This is a good UI font, but not at that 
size (and most "native" OS X applications indeed use a range of smaller sizes, 
depending on role.

It does have introduce a number of regressions, which the current patch aims to 
address. The most visible and problematic of these regressions is the loss of 
the Mac-style menu bar and thus of all menu items (actions).

The fix is straightforward : on OS X (and similarly affected platforms?), an 
instance of the native Cocoa platform theme is created through the private API, 
and used as a fallback rather than immediately falling back to the default 
implementations from `QPlatformTheme`. In addition, methods missing from (not 
overridden by) `KdePlatformTheme` are provided on OS X and call the 
corresponding methods from the native theme. It is this change which restores 
the menubar and even the Dock menu functionality.
One minor regression remains but should be easy to fix (elsewhere?): the 
Preferences menu loses its keyboard shortcut (Command-,).

Given the fallback nature of the native platform instance I have preferred to 
print a warning rather than using something like `qFatal`, above all because 
the message printed by qFatal tends to get lost on OS X. I can replace my use 
of `qWarning` with a dialog giving the choice between continuing or exiting the 
application - code that would be called in the menu methods because only there 
is it certain that the application actually needs a menubar.

In line with experience and feedback from the 

Re: Review Request 126198: [OS X] adaptations for the KdePlatformTheme (and autotests)

[René J . V . Bertin]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/126198/
---

(Updated Dec. 2, 2015, 10:42 p.m.)


Review request for KDE Software on Mac OS X, KDE Frameworks and Valorie 
Zimmerman.


Changes
---

apologies, files were missing from the previous patch.


Repository: frameworkintegration


Description
---

The KdePlatformTheme plugin can be enabled on OS X with a minimal patch to Qt5; 
all that is required is to include the `qgenericunixservices` and 
`qgenericunixthemes` components in the build, and to append `"kde"` to the list 
returned by `QCocoaIntegration::themeNames()` for instance under the condition 
that `KDE_SESSION_VERSION` is set to a suitable value in the environment.

This will allow KF5 and Qt5 applications to use the theme selected through KDE 
if FrameworkIntegration is installed and KDE_SESSION_VERSION is set, which 
seems like a sufficiently specific set of conditions that is easy to avoid by 
users who prefer to use the Mac native theme.

While requestion the KDE theme is also possible through `-style kde` or `env 
QT_STYLE_OVERRIDE=kde` the use of the KdePlatformTheme plugin appears to be the 
only way to get the full theme, including the font and colour selection. In my 
opinion it is above all the font customisation which is a very welcome feature 
for Qt/Mac; by default Qt applications use the default system font (Lucida 
Grande 13pt or even 14pt) throughout. This is a good UI font, but not at that 
size (and most "native" OS X applications indeed use a range of smaller sizes, 
depending on role.

It does have introduce a number of regressions, which the current patch aims to 
address. The most visible and problematic of these regressions is the loss of 
the Mac-style menu bar and thus of all menu items (actions).

The fix is straightforward : on OS X (and similarly affected platforms?), an 
instance of the native Cocoa platform theme is created through the private API, 
and used as a fallback rather than immediately falling back to the default 
implementations from `QPlatformTheme`. In addition, methods missing from (not 
overridden by) `KdePlatformTheme` are provided on OS X and call the 
corresponding methods from the native theme. It is this change which restores 
the menubar and even the Dock menu functionality.
One minor regression remains but should be easy to fix (elsewhere?): the 
Preferences menu loses its keyboard shortcut (Command-,).

Given the fallback nature of the native platform instance I have preferred to 
print a warning rather than using something like `qFatal`, above all because 
the message printed by qFatal tends to get lost on OS X. I can replace my use 
of `qWarning` with a dialog giving the choice between continuing or exiting the 
application - code that would be called in the menu methods because only there 
is it certain that the application actually needs a menubar.

In line with experience and feedback from the KDE(4)-Mac community I have 
decided to force the use of native dialogs rather than the ones from the 
KdePlatformPlugin.

In addition I set the fallback value for `ShowIconsOnPushButtons` to false in 
line with platform guidelines, and ensure that the autotests are not built as 
app bundles.


Diffs (updated)
-

  src/platformtheme/kdeplatformtheme.h 97d09df 
  src/platformtheme/kdemactheme.mm PRE-CREATION 
  src/platformtheme/kdemactheme.h PRE-CREATION 
  src/platformtheme/CMakeLists.txt 23f590e 
  src/kstyle/kstyle.mm PRE-CREATION 
  src/kstyle/CMakeLists.txt bc26667 
  autotests/CMakeLists.txt 7c2129c 
  src/platformtheme/kdeplatformtheme.cpp 80dbcb7 
  src/platformtheme/kfontsettingsdata.h 4b92c7d 
  src/platformtheme/kfontsettingsdatamac.h PRE-CREATION 
  src/platformtheme/kfontsettingsdatamac.mm PRE-CREATION 
  src/platformtheme/khintssettings.h ec064d3 
  src/platformtheme/khintssettingsmac.h PRE-CREATION 
  src/platformtheme/khintssettingsmac.mm PRE-CREATION 
  src/platformtheme/main_mac.cpp PRE-CREATION 

Diff: https://git.reviewboard.kde.org/r/126198/diff/


Testing
---

On Mac OS X with Qt 5.5.1, KF5 frameworks 5.16.0 and QtCurve git/head.

I have not verified to what extent my use of a private `QGuiApplication` API 
links builds to a specific Qt version (I consider that nothing shocking and a 
minor price to pay).
>>> Do I need to add some glue to the CMake file so that it will warn if the 
>>> private headerfiles are not available? Apparently no changes were required 
>>> to find them.


File Attachments


purely native OS X theme
  
https://git.reviewboard.kde.org/media/uploaded/files/2015/11/30/650d0da7-52b3-40d1-a1f9-cb610494cf77__Screen_Shot_2015-11-30_at_15.42.31.png
native theme but with `-style kde`
  

Re: Review Request 126198: [OS X] adaptations for the KdePlatformTheme (and autotests)

[Boudhayan Gupta]

> On Dec. 2, 2015, 3:15 p.m., Boudhayan Gupta wrote:
> > src/platformtheme/kdemactheme.mm, line 39
> > 
> >
> > This makes me very nervous.
> > 
> > Using private APIs is almost always a guarantee the application won't 
> > preserve binary compatibility even across point releases (eg Qt 5.5.0 - 
> > 5.5.1), let alone across major releases.
> > 
> > What makes it even more dangerous is that this file won't be built only 
> > on MacPorts/Fink but also by people making normal AppBundle releases - 
> > where you have no control over what versions of dependencies are being used.
> > 
> > Please try to find a public API alternative, even if it ends up being a 
> > giant hack. I once found a very elegant private API solution to making a 
> > QQuickWidget emit a release event on the mouse cursor, but just because of 
> > the whole binary compat problem (some Linux distros don't even ship apps 
> > depending on private headers) I had to create a dummy item inside the 
> > QtQuick code and interact with it to get the cursor released. Giant hack, 
> > but at least no private API usage.
> > 
> > Private API is **private** for a reason.
> 
> Luigi Toscano wrote:
> Afaik frameworkintegration is supposed to be an extension of the internal 
> Qt world and it has already been the case (I asked the same question few 
> Akademys ago). If you want to use QPA, you need to go down in the stack.
> 
> Martin Gräßlin wrote:
> yeah frameworkintegration is bound to use private API. So not really a 
> problem in this special case.
> 
> AFAIK some distros have adjusted the packaging to force a recompile for 
> new Qt releases.

Ah, that makes sense. Okay, I'm dropping this issue.


- Boudhayan


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/126198/#review89026
---


On Dec. 2, 2015, 1:59 a.m., René J.V. Bertin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126198/
> ---
> 
> (Updated Dec. 2, 2015, 1:59 a.m.)
> 
> 
> Review request for KDE Software on Mac OS X, KDE Frameworks and Valorie 
> Zimmerman.
> 
> 
> Repository: frameworkintegration
> 
> 
> Description
> ---
> 
> The KdePlatformTheme plugin can be enabled on OS X with a minimal patch to 
> Qt5; all that is required is to include the `qgenericunixservices` and 
> `qgenericunixthemes` components in the build, and to append `"kde"` to the 
> list returned by `QCocoaIntegration::themeNames()` for instance under the 
> condition that `KDE_SESSION_VERSION` is set to a suitable value in the 
> environment.
> 
> This will allow KF5 and Qt5 applications to use the theme selected through 
> KDE if FrameworkIntegration is installed and KDE_SESSION_VERSION is set, 
> which seems like a sufficiently specific set of conditions that is easy to 
> avoid by users who prefer to use the Mac native theme.
> 
> While requestion the KDE theme is also possible through `-style kde` or `env 
> QT_STYLE_OVERRIDE=kde` the use of the KdePlatformTheme plugin appears to be 
> the only way to get the full theme, including the font and colour selection. 
> In my opinion it is above all the font customisation which is a very welcome 
> feature for Qt/Mac; by default Qt applications use the default system font 
> (Lucida Grande 13pt or even 14pt) throughout. This is a good UI font, but not 
> at that size (and most "native" OS X applications indeed use a range of 
> smaller sizes, depending on role.
> 
> It does have introduce a number of regressions, which the current patch aims 
> to address. The most visible and problematic of these regressions is the loss 
> of the Mac-style menu bar and thus of all menu items (actions).
> 
> The fix is straightforward : on OS X (and similarly affected platforms?), an 
> instance of the native Cocoa platform theme is created through the private 
> API, and used as a fallback rather than immediately falling back to the 
> default implementations from `QPlatformTheme`. In addition, methods missing 
> from (not overridden by) `KdePlatformTheme` are provided on OS X and call the 
> corresponding methods from the native theme. It is this change which restores 
> the menubar and even the Dock menu functionality.
> One minor regression remains but should be easy to fix (elsewhere?): the 
> Preferences menu loses its keyboard shortcut (Command-,).
> 
> Given the fallback nature of the native platform instance I have preferred to 
> print a warning rather than using something like `qFatal`, above all because 
> the message printed by qFatal tends to get lost on OS X. I can replace my use 
> of `qWarning` with 

Re: Review Request 126198: [OS X] adaptations for the KdePlatformTheme (and autotests)

[Boudhayan Gupta]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/126198/#review89026
---



src/platformtheme/kdemactheme.mm (line 39)


This makes me very nervous.

Using private APIs is almost always a guarantee the application won't 
preserve binary compatibility even across point releases (eg Qt 5.5.0 - 5.5.1), 
let alone across major releases.

What makes it even more dangerous is that this file won't be built only on 
MacPorts/Fink but also by people making normal AppBundle releases - where you 
have no control over what versions of dependencies are being used.

Please try to find a public API alternative, even if it ends up being a 
giant hack. I once found a very elegant private API solution to making a 
QQuickWidget emit a release event on the mouse cursor, but just because of the 
whole binary compat problem (some Linux distros don't even ship apps depending 
on private headers) I had to create a dummy item inside the QtQuick code and 
interact with it to get the cursor released. Giant hack, but at least no 
private API usage.

Private API is **private** for a reason.


- Boudhayan Gupta


On Dec. 2, 2015, 1:59 a.m., René J.V. Bertin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126198/
> ---
> 
> (Updated Dec. 2, 2015, 1:59 a.m.)
> 
> 
> Review request for KDE Software on Mac OS X, KDE Frameworks and Valorie 
> Zimmerman.
> 
> 
> Repository: frameworkintegration
> 
> 
> Description
> ---
> 
> The KdePlatformTheme plugin can be enabled on OS X with a minimal patch to 
> Qt5; all that is required is to include the `qgenericunixservices` and 
> `qgenericunixthemes` components in the build, and to append `"kde"` to the 
> list returned by `QCocoaIntegration::themeNames()` for instance under the 
> condition that `KDE_SESSION_VERSION` is set to a suitable value in the 
> environment.
> 
> This will allow KF5 and Qt5 applications to use the theme selected through 
> KDE if FrameworkIntegration is installed and KDE_SESSION_VERSION is set, 
> which seems like a sufficiently specific set of conditions that is easy to 
> avoid by users who prefer to use the Mac native theme.
> 
> While requestion the KDE theme is also possible through `-style kde` or `env 
> QT_STYLE_OVERRIDE=kde` the use of the KdePlatformTheme plugin appears to be 
> the only way to get the full theme, including the font and colour selection. 
> In my opinion it is above all the font customisation which is a very welcome 
> feature for Qt/Mac; by default Qt applications use the default system font 
> (Lucida Grande 13pt or even 14pt) throughout. This is a good UI font, but not 
> at that size (and most "native" OS X applications indeed use a range of 
> smaller sizes, depending on role.
> 
> It does have introduce a number of regressions, which the current patch aims 
> to address. The most visible and problematic of these regressions is the loss 
> of the Mac-style menu bar and thus of all menu items (actions).
> 
> The fix is straightforward : on OS X (and similarly affected platforms?), an 
> instance of the native Cocoa platform theme is created through the private 
> API, and used as a fallback rather than immediately falling back to the 
> default implementations from `QPlatformTheme`. In addition, methods missing 
> from (not overridden by) `KdePlatformTheme` are provided on OS X and call the 
> corresponding methods from the native theme. It is this change which restores 
> the menubar and even the Dock menu functionality.
> One minor regression remains but should be easy to fix (elsewhere?): the 
> Preferences menu loses its keyboard shortcut (Command-,).
> 
> Given the fallback nature of the native platform instance I have preferred to 
> print a warning rather than using something like `qFatal`, above all because 
> the message printed by qFatal tends to get lost on OS X. I can replace my use 
> of `qWarning` with a dialog giving the choice between continuing or exiting 
> the application - code that would be called in the menu methods because only 
> there is it certain that the application actually needs a menubar.
> 
> In line with experience and feedback from the KDE(4)-Mac community I have 
> decided to force the use of native dialogs rather than the ones from the 
> KdePlatformPlugin.
> 
> In addition I set the fallback value for `ShowIconsOnPushButtons` to false in 
> line with platform guidelines, and ensure that the autotests are not built as 
> app bundles.
> 
> 
> Diffs
> -
> 
>   autotests/CMakeLists.txt 7c2129c 
>   src/kstyle/CMakeLists.txt bc26667 
>   src/kstyle/kstyle.mm PRE-CREATION 
>   

Re: Review Request 126198: [OS X] adaptations for the KdePlatformTheme (and autotests)

[Luigi Toscano]

> On Dic. 2, 2015, 10:45 a.m., Boudhayan Gupta wrote:
> > src/platformtheme/kdemactheme.mm, line 39
> > 
> >
> > This makes me very nervous.
> > 
> > Using private APIs is almost always a guarantee the application won't 
> > preserve binary compatibility even across point releases (eg Qt 5.5.0 - 
> > 5.5.1), let alone across major releases.
> > 
> > What makes it even more dangerous is that this file won't be built only 
> > on MacPorts/Fink but also by people making normal AppBundle releases - 
> > where you have no control over what versions of dependencies are being used.
> > 
> > Please try to find a public API alternative, even if it ends up being a 
> > giant hack. I once found a very elegant private API solution to making a 
> > QQuickWidget emit a release event on the mouse cursor, but just because of 
> > the whole binary compat problem (some Linux distros don't even ship apps 
> > depending on private headers) I had to create a dummy item inside the 
> > QtQuick code and interact with it to get the cursor released. Giant hack, 
> > but at least no private API usage.
> > 
> > Private API is **private** for a reason.

Afaik frameworkintegration is supposed to be an extension of the internal Qt 
world and it has already been the case (I asked the same question few Akademys 
ago). If you want to use QPA, you need to go down in the stack.


- Luigi


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/126198/#review89026
---


On Dic. 1, 2015, 9:29 p.m., René J.V. Bertin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126198/
> ---
> 
> (Updated Dic. 1, 2015, 9:29 p.m.)
> 
> 
> Review request for KDE Software on Mac OS X, KDE Frameworks and Valorie 
> Zimmerman.
> 
> 
> Repository: frameworkintegration
> 
> 
> Description
> ---
> 
> The KdePlatformTheme plugin can be enabled on OS X with a minimal patch to 
> Qt5; all that is required is to include the `qgenericunixservices` and 
> `qgenericunixthemes` components in the build, and to append `"kde"` to the 
> list returned by `QCocoaIntegration::themeNames()` for instance under the 
> condition that `KDE_SESSION_VERSION` is set to a suitable value in the 
> environment.
> 
> This will allow KF5 and Qt5 applications to use the theme selected through 
> KDE if FrameworkIntegration is installed and KDE_SESSION_VERSION is set, 
> which seems like a sufficiently specific set of conditions that is easy to 
> avoid by users who prefer to use the Mac native theme.
> 
> While requestion the KDE theme is also possible through `-style kde` or `env 
> QT_STYLE_OVERRIDE=kde` the use of the KdePlatformTheme plugin appears to be 
> the only way to get the full theme, including the font and colour selection. 
> In my opinion it is above all the font customisation which is a very welcome 
> feature for Qt/Mac; by default Qt applications use the default system font 
> (Lucida Grande 13pt or even 14pt) throughout. This is a good UI font, but not 
> at that size (and most "native" OS X applications indeed use a range of 
> smaller sizes, depending on role.
> 
> It does have introduce a number of regressions, which the current patch aims 
> to address. The most visible and problematic of these regressions is the loss 
> of the Mac-style menu bar and thus of all menu items (actions).
> 
> The fix is straightforward : on OS X (and similarly affected platforms?), an 
> instance of the native Cocoa platform theme is created through the private 
> API, and used as a fallback rather than immediately falling back to the 
> default implementations from `QPlatformTheme`. In addition, methods missing 
> from (not overridden by) `KdePlatformTheme` are provided on OS X and call the 
> corresponding methods from the native theme. It is this change which restores 
> the menubar and even the Dock menu functionality.
> One minor regression remains but should be easy to fix (elsewhere?): the 
> Preferences menu loses its keyboard shortcut (Command-,).
> 
> Given the fallback nature of the native platform instance I have preferred to 
> print a warning rather than using something like `qFatal`, above all because 
> the message printed by qFatal tends to get lost on OS X. I can replace my use 
> of `qWarning` with a dialog giving the choice between continuing or exiting 
> the application - code that would be called in the menu methods because only 
> there is it certain that the application actually needs a menubar.
> 
> In line with experience and feedback from the KDE(4)-Mac community I have 
> decided to force the use of native dialogs 

Re: Review Request 126198: [OS X] adaptations for the KdePlatformTheme (and autotests)

[Martin Gräßlin]

> On Dec. 2, 2015, 10:45 a.m., Boudhayan Gupta wrote:
> > src/platformtheme/kdemactheme.mm, line 39
> > 
> >
> > This makes me very nervous.
> > 
> > Using private APIs is almost always a guarantee the application won't 
> > preserve binary compatibility even across point releases (eg Qt 5.5.0 - 
> > 5.5.1), let alone across major releases.
> > 
> > What makes it even more dangerous is that this file won't be built only 
> > on MacPorts/Fink but also by people making normal AppBundle releases - 
> > where you have no control over what versions of dependencies are being used.
> > 
> > Please try to find a public API alternative, even if it ends up being a 
> > giant hack. I once found a very elegant private API solution to making a 
> > QQuickWidget emit a release event on the mouse cursor, but just because of 
> > the whole binary compat problem (some Linux distros don't even ship apps 
> > depending on private headers) I had to create a dummy item inside the 
> > QtQuick code and interact with it to get the cursor released. Giant hack, 
> > but at least no private API usage.
> > 
> > Private API is **private** for a reason.
> 
> Luigi Toscano wrote:
> Afaik frameworkintegration is supposed to be an extension of the internal 
> Qt world and it has already been the case (I asked the same question few 
> Akademys ago). If you want to use QPA, you need to go down in the stack.

yeah frameworkintegration is bound to use private API. So not really a problem 
in this special case.

AFAIK some distros have adjusted the packaging to force a recompile for new Qt 
releases.


- Martin


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/126198/#review89026
---


On Dec. 1, 2015, 9:29 p.m., René J.V. Bertin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126198/
> ---
> 
> (Updated Dec. 1, 2015, 9:29 p.m.)
> 
> 
> Review request for KDE Software on Mac OS X, KDE Frameworks and Valorie 
> Zimmerman.
> 
> 
> Repository: frameworkintegration
> 
> 
> Description
> ---
> 
> The KdePlatformTheme plugin can be enabled on OS X with a minimal patch to 
> Qt5; all that is required is to include the `qgenericunixservices` and 
> `qgenericunixthemes` components in the build, and to append `"kde"` to the 
> list returned by `QCocoaIntegration::themeNames()` for instance under the 
> condition that `KDE_SESSION_VERSION` is set to a suitable value in the 
> environment.
> 
> This will allow KF5 and Qt5 applications to use the theme selected through 
> KDE if FrameworkIntegration is installed and KDE_SESSION_VERSION is set, 
> which seems like a sufficiently specific set of conditions that is easy to 
> avoid by users who prefer to use the Mac native theme.
> 
> While requestion the KDE theme is also possible through `-style kde` or `env 
> QT_STYLE_OVERRIDE=kde` the use of the KdePlatformTheme plugin appears to be 
> the only way to get the full theme, including the font and colour selection. 
> In my opinion it is above all the font customisation which is a very welcome 
> feature for Qt/Mac; by default Qt applications use the default system font 
> (Lucida Grande 13pt or even 14pt) throughout. This is a good UI font, but not 
> at that size (and most "native" OS X applications indeed use a range of 
> smaller sizes, depending on role.
> 
> It does have introduce a number of regressions, which the current patch aims 
> to address. The most visible and problematic of these regressions is the loss 
> of the Mac-style menu bar and thus of all menu items (actions).
> 
> The fix is straightforward : on OS X (and similarly affected platforms?), an 
> instance of the native Cocoa platform theme is created through the private 
> API, and used as a fallback rather than immediately falling back to the 
> default implementations from `QPlatformTheme`. In addition, methods missing 
> from (not overridden by) `KdePlatformTheme` are provided on OS X and call the 
> corresponding methods from the native theme. It is this change which restores 
> the menubar and even the Dock menu functionality.
> One minor regression remains but should be easy to fix (elsewhere?): the 
> Preferences menu loses its keyboard shortcut (Command-,).
> 
> Given the fallback nature of the native platform instance I have preferred to 
> print a warning rather than using something like `qFatal`, above all because 
> the message printed by qFatal tends to get lost on OS X. I can replace my use 
> of `qWarning` with a dialog giving the choice between continuing or exiting 
> the application - code that would be called 

Re: Review Request 126198: [OS X] adaptations for the KdePlatformTheme (and autotests)

[René J . V . Bertin]

> On Dec. 2, 2015, 8:38 a.m., Martin Gräßlin wrote:
> > Just wondering: if your main aim is to change default settings, why don't 
> > follow what linux distros do? That is ship some default configuration 
> > interface. For inspiration check e.g. kubuntu-default-settings.
> 
> René J.V. Bertin wrote:
> If I understand you correctly, you're suggesting I write a set of default 
> configuration values that could go into a look-and-feel package (?) I 
> stripped support for those in an effort to be PC because they're expected in 
> a folder called `plasma`.
> 
> It does look like this idea could remove the need for a Mac-specific 
> KHintsSettings class.
> 
> One thing I don't understand here: precedence between the package 
> configured in kdeglobals (`LookAndFeelPackage`) which is stored in 
> `mDefaultLnfConfig` and the hardcoded `defaultLookAndFeelPackage` package. 
> The latter is stored in `mLnfConfig` if different from the user-selected 
> package, but then `readConfigValue()` gives precedence to `mLnfConfig` if 
> it's set. That looks as if the effective roles are reversed.
> 
> René J.V. Bertin wrote:
> Too fast again :-/
> 
> I see that my suggestion below to check `mDefaultLnfConfig` before 
> accessing it is moot, but shouldn't `mLnfConfig` be initialised to 0 if 
> `looknfeel == defaultLookAndFeelPackage`?
> 
> Martin Gräßlin wrote:
> > If I understand you correctly, you're suggesting I write a set of 
> default configuration values that could go into a look-and-feel package 
> 
> kconfig files. You just install a more global config file with your 
> default values. Best check with some distros on how to do that. (I'm from the 
> opposite department, telling distros when they broke it :-P )

Ah, ok. For now I've been focussing on your other suggestion, using inheritance.
But in a sense I'd prefer using default values determined at runtime for the 
"perfect" native look; either using Qt calls as I've done in my latest attempt, 
or using calls into whatever native API is available for that purpose. That's 
why I've been using ObjC++ after all :)


- René J.V.


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/126198/#review89019
---


On Dec. 2, 2015, 10:38 p.m., René J.V. Bertin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126198/
> ---
> 
> (Updated Dec. 2, 2015, 10:38 p.m.)
> 
> 
> Review request for KDE Software on Mac OS X, KDE Frameworks and Valorie 
> Zimmerman.
> 
> 
> Repository: frameworkintegration
> 
> 
> Description
> ---
> 
> The KdePlatformTheme plugin can be enabled on OS X with a minimal patch to 
> Qt5; all that is required is to include the `qgenericunixservices` and 
> `qgenericunixthemes` components in the build, and to append `"kde"` to the 
> list returned by `QCocoaIntegration::themeNames()` for instance under the 
> condition that `KDE_SESSION_VERSION` is set to a suitable value in the 
> environment.
> 
> This will allow KF5 and Qt5 applications to use the theme selected through 
> KDE if FrameworkIntegration is installed and KDE_SESSION_VERSION is set, 
> which seems like a sufficiently specific set of conditions that is easy to 
> avoid by users who prefer to use the Mac native theme.
> 
> While requestion the KDE theme is also possible through `-style kde` or `env 
> QT_STYLE_OVERRIDE=kde` the use of the KdePlatformTheme plugin appears to be 
> the only way to get the full theme, including the font and colour selection. 
> In my opinion it is above all the font customisation which is a very welcome 
> feature for Qt/Mac; by default Qt applications use the default system font 
> (Lucida Grande 13pt or even 14pt) throughout. This is a good UI font, but not 
> at that size (and most "native" OS X applications indeed use a range of 
> smaller sizes, depending on role.
> 
> It does have introduce a number of regressions, which the current patch aims 
> to address. The most visible and problematic of these regressions is the loss 
> of the Mac-style menu bar and thus of all menu items (actions).
> 
> The fix is straightforward : on OS X (and similarly affected platforms?), an 
> instance of the native Cocoa platform theme is created through the private 
> API, and used as a fallback rather than immediately falling back to the 
> default implementations from `QPlatformTheme`. In addition, methods missing 
> from (not overridden by) `KdePlatformTheme` are provided on OS X and call the 
> corresponding methods from the native theme. It is this change which restores 
> the menubar and even the Dock menu functionality.
> One minor regression remains but should be easy to fix (elsewhere?): the 
> 

Re: Review Request 126198: [OS X] adaptations for the KdePlatformTheme (and autotests)

[René J . V . Bertin]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/126198/#review89008
---


A couple of things I jotted down today, probably open doors but nonetheless 
maybe something to consider and take into account for those who have little or 
no first-hand experience with Qt on OS X or even MS Windows.

On Linux and other Unices that aren't OS X/iOS, there is no native platform SDK 
that provides predefined generic (and less generic) buttons, menus and whatever 
widgets. If we ignore Wayland, there was only X11, which simply provides 
windows, undecorated and empty bits of screen space that aren't even 
necessarily rectangular. What you do with those windows is completely up to 
you. And that's exactly what Qt did when they designed the Fusion theme that 
was introduced in Qt5.
Qt applications that are started on a Linux system and that don't have access 
to anything else, will by default use the QGenericUnixTheme, combined with 
Fusion. Given that this was designed from scratch, it is not surprising that 
its use gives interfaces designed with the generic Qt toolbox a pretty damn 
good look, leading to interfaces that are perfectly workable and maybe even 
enjoyable to use. They even don't look too much out of place on other 
platforms, because that was another design criterium from what I understand.

OS X and MS Windows on the other hand do provide their own SDKs with predefined 
generic (and less generic) buttons, menus and whatever widgets. Cross-platform 
middlewares like Qt can indeed be expected to use those SDKs as the basis for 
their platform implementation (which doesn't mean they should be forbidden from 
also providing a true cross-platform-coherent look). But such a middleware can 
only guarantee to provide generic controls that map to the generic native 
widgets, or else at most a smallest common denominator collection of more 
specialised widgets that exist in the SDKs of all supported platforms.
As a result, interfaces built with only that cross-platform toolbox will look 
more or less good depending on how well generic design principles that come 
with using Qt map to the underlying toolkits --- and influenced by Tufte as I 
am, I firmly believe that this can and will have implications for interface 
usability.

To borrow a metaphor and proverb: a generic, "bare-bones" Qt interface using 
the OS X native theme will look like it's wearing the emperor's clothes, but 
clearly wasn't designed for wearing those clothes. The extent to which this is 
the case varies from application to application, but you can (and likely will) 
end up with cases where the native theme makes an application look more out of 
place than a theme like Fusion (if it could be used in its full form).

This was particularly visible in KDE4 applications using the native "macintosh" 
theme/style (enough examples of kate and kcalc, among others, have been 
posted). It looks like the underlying code hasn't that much evolved since Qt 
4.8.7, so I'm expecting certain issues we identified in KDE4 apps to exist 
under KF5 too. (Indeed, the font issue with the tab selector widget looks very 
familiar.)

- René J.V. Bertin


On Dec. 1, 2015, 9:29 p.m., René J.V. Bertin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126198/
> ---
> 
> (Updated Dec. 1, 2015, 9:29 p.m.)
> 
> 
> Review request for KDE Software on Mac OS X, KDE Frameworks and Valorie 
> Zimmerman.
> 
> 
> Repository: frameworkintegration
> 
> 
> Description
> ---
> 
> The KdePlatformTheme plugin can be enabled on OS X with a minimal patch to 
> Qt5; all that is required is to include the `qgenericunixservices` and 
> `qgenericunixthemes` components in the build, and to append `"kde"` to the 
> list returned by `QCocoaIntegration::themeNames()` for instance under the 
> condition that `KDE_SESSION_VERSION` is set to a suitable value in the 
> environment.
> 
> This will allow KF5 and Qt5 applications to use the theme selected through 
> KDE if FrameworkIntegration is installed and KDE_SESSION_VERSION is set, 
> which seems like a sufficiently specific set of conditions that is easy to 
> avoid by users who prefer to use the Mac native theme.
> 
> While requestion the KDE theme is also possible through `-style kde` or `env 
> QT_STYLE_OVERRIDE=kde` the use of the KdePlatformTheme plugin appears to be 
> the only way to get the full theme, including the font and colour selection. 
> In my opinion it is above all the font customisation which is a very welcome 
> feature for Qt/Mac; by default Qt applications use the default system font 
> (Lucida Grande 13pt or even 14pt) throughout. This is a good UI font, but not 
> at that size (and most 

Re: Review Request 126198: [OS X] adaptations for the KdePlatformTheme (and autotests)

[René J . V . Bertin]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/126198/
---

(Updated Dec. 1, 2015, 9:29 p.m.)


Review request for KDE Software on Mac OS X, KDE Frameworks and Valorie 
Zimmerman.


Changes
---

This is the promised new revision, which introduces Mac-specific files, with 
Mac-specific KdeMacTheme and KHintSettingsMac classes. I stripped them of 
everything I saw that was either irrelevant or unsupported.

I'm not so sure what to make of the other components; I don't see anything 
Plasma-specific in InfoPage, the KMessageBox seems just as useful on OS X as 
everywhere else; the same could probably be said about KF5Style if I understand 
its mission correctly.


Repository: frameworkintegration


Description
---

The KdePlatformTheme plugin can be enabled on OS X with a minimal patch to Qt5; 
all that is required is to include the `qgenericunixservices` and 
`qgenericunixthemes` components in the build, and to append `"kde"` to the list 
returned by `QCocoaIntegration::themeNames()` for instance under the condition 
that `KDE_SESSION_VERSION` is set to a suitable value in the environment.

This will allow KF5 and Qt5 applications to use the theme selected through KDE 
if FrameworkIntegration is installed and KDE_SESSION_VERSION is set, which 
seems like a sufficiently specific set of conditions that is easy to avoid by 
users who prefer to use the Mac native theme.

While requestion the KDE theme is also possible through `-style kde` or `env 
QT_STYLE_OVERRIDE=kde` the use of the KdePlatformTheme plugin appears to be the 
only way to get the full theme, including the font and colour selection. In my 
opinion it is above all the font customisation which is a very welcome feature 
for Qt/Mac; by default Qt applications use the default system font (Lucida 
Grande 13pt or even 14pt) throughout. This is a good UI font, but not at that 
size (and most "native" OS X applications indeed use a range of smaller sizes, 
depending on role.

It does have introduce a number of regressions, which the current patch aims to 
address. The most visible and problematic of these regressions is the loss of 
the Mac-style menu bar and thus of all menu items (actions).

The fix is straightforward : on OS X (and similarly affected platforms?), an 
instance of the native Cocoa platform theme is created through the private API, 
and used as a fallback rather than immediately falling back to the default 
implementations from `QPlatformTheme`. In addition, methods missing from (not 
overridden by) `KdePlatformTheme` are provided on OS X and call the 
corresponding methods from the native theme. It is this change which restores 
the menubar and even the Dock menu functionality.
One minor regression remains but should be easy to fix (elsewhere?): the 
Preferences menu loses its keyboard shortcut (Command-,).

Given the fallback nature of the native platform instance I have preferred to 
print a warning rather than using something like `qFatal`, above all because 
the message printed by qFatal tends to get lost on OS X. I can replace my use 
of `qWarning` with a dialog giving the choice between continuing or exiting the 
application - code that would be called in the menu methods because only there 
is it certain that the application actually needs a menubar.

In line with experience and feedback from the KDE(4)-Mac community I have 
decided to force the use of native dialogs rather than the ones from the 
KdePlatformPlugin.

In addition I set the fallback value for `ShowIconsOnPushButtons` to false in 
line with platform guidelines, and ensure that the autotests are not built as 
app bundles.


Diffs (updated)
-

  autotests/CMakeLists.txt 7c2129c 
  src/kstyle/CMakeLists.txt bc26667 
  src/kstyle/kstyle.mm PRE-CREATION 
  src/platformtheme/CMakeLists.txt 23f590e 
  src/platformtheme/kdemactheme.h PRE-CREATION 
  src/platformtheme/kdemactheme.mm PRE-CREATION 
  src/platformtheme/khintssettingsmac.h PRE-CREATION 
  src/platformtheme/khintssettingsmac.mm PRE-CREATION 
  src/platformtheme/main_mac.cpp PRE-CREATION 

Diff: https://git.reviewboard.kde.org/r/126198/diff/


Testing
---

On Mac OS X with Qt 5.5.1, KF5 frameworks 5.16.0 and QtCurve git/head.

I have not verified to what extent my use of a private `QGuiApplication` API 
links builds to a specific Qt version (I consider that nothing shocking and a 
minor price to pay).
>>> Do I need to add some glue to the CMake file so that it will warn if the 
>>> private headerfiles are not available? Apparently no changes were required 
>>> to find them.


File Attachments


purely native OS X theme
  
https://git.reviewboard.kde.org/media/uploaded/files/2015/11/30/650d0da7-52b3-40d1-a1f9-cb610494cf77__Screen_Shot_2015-11-30_at_15.42.31.png
native theme but with `-style kde`
  

Re: Review Request 126198: [OS X] adaptations for the KdePlatformTheme (and autotests)

[René J . V . Bertin]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/126198/#review89009
---



src/platformtheme/kdemactheme.mm (lines 53 - 87)


I thought it would be best to use a native dialog here to show a warning 
dialog, but it turns out there is already a native event filter in place that 
causes a conflict and a crash.
I'll have to replace this with a QMessageBox.


- René J.V. Bertin


On Dec. 1, 2015, 9:29 p.m., René J.V. Bertin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126198/
> ---
> 
> (Updated Dec. 1, 2015, 9:29 p.m.)
> 
> 
> Review request for KDE Software on Mac OS X, KDE Frameworks and Valorie 
> Zimmerman.
> 
> 
> Repository: frameworkintegration
> 
> 
> Description
> ---
> 
> The KdePlatformTheme plugin can be enabled on OS X with a minimal patch to 
> Qt5; all that is required is to include the `qgenericunixservices` and 
> `qgenericunixthemes` components in the build, and to append `"kde"` to the 
> list returned by `QCocoaIntegration::themeNames()` for instance under the 
> condition that `KDE_SESSION_VERSION` is set to a suitable value in the 
> environment.
> 
> This will allow KF5 and Qt5 applications to use the theme selected through 
> KDE if FrameworkIntegration is installed and KDE_SESSION_VERSION is set, 
> which seems like a sufficiently specific set of conditions that is easy to 
> avoid by users who prefer to use the Mac native theme.
> 
> While requestion the KDE theme is also possible through `-style kde` or `env 
> QT_STYLE_OVERRIDE=kde` the use of the KdePlatformTheme plugin appears to be 
> the only way to get the full theme, including the font and colour selection. 
> In my opinion it is above all the font customisation which is a very welcome 
> feature for Qt/Mac; by default Qt applications use the default system font 
> (Lucida Grande 13pt or even 14pt) throughout. This is a good UI font, but not 
> at that size (and most "native" OS X applications indeed use a range of 
> smaller sizes, depending on role.
> 
> It does have introduce a number of regressions, which the current patch aims 
> to address. The most visible and problematic of these regressions is the loss 
> of the Mac-style menu bar and thus of all menu items (actions).
> 
> The fix is straightforward : on OS X (and similarly affected platforms?), an 
> instance of the native Cocoa platform theme is created through the private 
> API, and used as a fallback rather than immediately falling back to the 
> default implementations from `QPlatformTheme`. In addition, methods missing 
> from (not overridden by) `KdePlatformTheme` are provided on OS X and call the 
> corresponding methods from the native theme. It is this change which restores 
> the menubar and even the Dock menu functionality.
> One minor regression remains but should be easy to fix (elsewhere?): the 
> Preferences menu loses its keyboard shortcut (Command-,).
> 
> Given the fallback nature of the native platform instance I have preferred to 
> print a warning rather than using something like `qFatal`, above all because 
> the message printed by qFatal tends to get lost on OS X. I can replace my use 
> of `qWarning` with a dialog giving the choice between continuing or exiting 
> the application - code that would be called in the menu methods because only 
> there is it certain that the application actually needs a menubar.
> 
> In line with experience and feedback from the KDE(4)-Mac community I have 
> decided to force the use of native dialogs rather than the ones from the 
> KdePlatformPlugin.
> 
> In addition I set the fallback value for `ShowIconsOnPushButtons` to false in 
> line with platform guidelines, and ensure that the autotests are not built as 
> app bundles.
> 
> 
> Diffs
> -
> 
>   autotests/CMakeLists.txt 7c2129c 
>   src/kstyle/CMakeLists.txt bc26667 
>   src/kstyle/kstyle.mm PRE-CREATION 
>   src/platformtheme/CMakeLists.txt 23f590e 
>   src/platformtheme/kdemactheme.h PRE-CREATION 
>   src/platformtheme/kdemactheme.mm PRE-CREATION 
>   src/platformtheme/khintssettingsmac.h PRE-CREATION 
>   src/platformtheme/khintssettingsmac.mm PRE-CREATION 
>   src/platformtheme/main_mac.cpp PRE-CREATION 
> 
> Diff: https://git.reviewboard.kde.org/r/126198/diff/
> 
> 
> Testing
> ---
> 
> On Mac OS X with Qt 5.5.1, KF5 frameworks 5.16.0 and QtCurve git/head.
> 
> I have not verified to what extent my use of a private `QGuiApplication` API 
> links builds to a specific Qt version (I consider that nothing shocking and a 
> minor price to pay).
> >>> Do I need to add some glue to the CMake file so that it will warn if the 
> >>> private 

Re: Review Request 126198: [OS X] adaptations for the KdePlatformTheme (and autotests)

[Martin Gräßlin]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/126198/#review89021
---


Overall I think this is now too much code duplication. With this appraoch you 
don't get bug fixes from the base code. I recommed to rather go for inheritance 
to have the actual code which can be shared still together.


src/platformtheme/CMakeLists.txt (lines 22 - 46)


please change this part to only contain differences. Otherwise it becomes 
difficult to maintain.

Like

set(platformtheme_SRCS ...)
if (APPLE)
set(platformtheme_SRCS ${platformtheme_SRCS} foo.mm)
endif()


Ideally I also don't want the generic part in the else branch. This should 
be more a feature based approach. What I don't want is that we generate a setup 
where it goes if(APPLE) else if (WINDOWS) else if (ANDROID)... You get what I 
mean ;-)



src/platformtheme/CMakeLists.txt (line 46)


please use the newer and easier endif() variant.


- Martin Gräßlin


On Dec. 1, 2015, 9:29 p.m., René J.V. Bertin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126198/
> ---
> 
> (Updated Dec. 1, 2015, 9:29 p.m.)
> 
> 
> Review request for KDE Software on Mac OS X, KDE Frameworks and Valorie 
> Zimmerman.
> 
> 
> Repository: frameworkintegration
> 
> 
> Description
> ---
> 
> The KdePlatformTheme plugin can be enabled on OS X with a minimal patch to 
> Qt5; all that is required is to include the `qgenericunixservices` and 
> `qgenericunixthemes` components in the build, and to append `"kde"` to the 
> list returned by `QCocoaIntegration::themeNames()` for instance under the 
> condition that `KDE_SESSION_VERSION` is set to a suitable value in the 
> environment.
> 
> This will allow KF5 and Qt5 applications to use the theme selected through 
> KDE if FrameworkIntegration is installed and KDE_SESSION_VERSION is set, 
> which seems like a sufficiently specific set of conditions that is easy to 
> avoid by users who prefer to use the Mac native theme.
> 
> While requestion the KDE theme is also possible through `-style kde` or `env 
> QT_STYLE_OVERRIDE=kde` the use of the KdePlatformTheme plugin appears to be 
> the only way to get the full theme, including the font and colour selection. 
> In my opinion it is above all the font customisation which is a very welcome 
> feature for Qt/Mac; by default Qt applications use the default system font 
> (Lucida Grande 13pt or even 14pt) throughout. This is a good UI font, but not 
> at that size (and most "native" OS X applications indeed use a range of 
> smaller sizes, depending on role.
> 
> It does have introduce a number of regressions, which the current patch aims 
> to address. The most visible and problematic of these regressions is the loss 
> of the Mac-style menu bar and thus of all menu items (actions).
> 
> The fix is straightforward : on OS X (and similarly affected platforms?), an 
> instance of the native Cocoa platform theme is created through the private 
> API, and used as a fallback rather than immediately falling back to the 
> default implementations from `QPlatformTheme`. In addition, methods missing 
> from (not overridden by) `KdePlatformTheme` are provided on OS X and call the 
> corresponding methods from the native theme. It is this change which restores 
> the menubar and even the Dock menu functionality.
> One minor regression remains but should be easy to fix (elsewhere?): the 
> Preferences menu loses its keyboard shortcut (Command-,).
> 
> Given the fallback nature of the native platform instance I have preferred to 
> print a warning rather than using something like `qFatal`, above all because 
> the message printed by qFatal tends to get lost on OS X. I can replace my use 
> of `qWarning` with a dialog giving the choice between continuing or exiting 
> the application - code that would be called in the menu methods because only 
> there is it certain that the application actually needs a menubar.
> 
> In line with experience and feedback from the KDE(4)-Mac community I have 
> decided to force the use of native dialogs rather than the ones from the 
> KdePlatformPlugin.
> 
> In addition I set the fallback value for `ShowIconsOnPushButtons` to false in 
> line with platform guidelines, and ensure that the autotests are not built as 
> app bundles.
> 
> 
> Diffs
> -
> 
>   autotests/CMakeLists.txt 7c2129c 
>   src/kstyle/CMakeLists.txt bc26667 
>   src/kstyle/kstyle.mm PRE-CREATION 
>   src/platformtheme/CMakeLists.txt 23f590e 
>   src/platformtheme/kdemactheme.h 

Re: Review Request 126198: [OS X] adaptations for the KdePlatformTheme (and autotests)

[René J . V . Bertin]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/126198/
---

(Updated Dec. 1, 2015, 2:03 p.m.)


Review request for KDE Software on Mac OS X, KDE Frameworks and Valorie 
Zimmerman.


Changes
---

I'll be uploading a revised patch later, but in the meantime here's what you 
get when the KDEPlatformThemePlugin is combined with the native theme (actually 
called `macintosh` and not `Macintosh (aqua)` as I thought earlier).

This is of course the theme that a user would get who installs KF5 for the 
first time, and doesn't use systemsettings5 (nor a kdeglobals file from his/her 
Linux desktop). It's *almost* what I would expect from a plugin that extends 
the native plugin with support for different fonts/font roles, palettes and 
icon sets without otherwise changing widget appearance.

The interface doesn't become noticeably more compact, but that's because the 
application itself imposes a minimum size on certain of its elements without 
taking the actual content dimensions into account.


Repository: frameworkintegration


Description
---

The KdePlatformTheme plugin can be enabled on OS X with a minimal patch to Qt5; 
all that is required is to include the `qgenericunixservices` and 
`qgenericunixthemes` components in the build, and to append `"kde"` to the list 
returned by `QCocoaIntegration::themeNames()` for instance under the condition 
that `KDE_SESSION_VERSION` is set to a suitable value in the environment.

This will allow KF5 and Qt5 applications to use the theme selected through KDE 
if FrameworkIntegration is installed and KDE_SESSION_VERSION is set, which 
seems like a sufficiently specific set of conditions that is easy to avoid by 
users who prefer to use the Mac native theme.

While requestion the KDE theme is also possible through `-style kde` or `env 
QT_STYLE_OVERRIDE=kde` the use of the KdePlatformTheme plugin appears to be the 
only way to get the full theme, including the font and colour selection. In my 
opinion it is above all the font customisation which is a very welcome feature 
for Qt/Mac; by default Qt applications use the default system font (Lucida 
Grande 13pt or even 14pt) throughout. This is a good UI font, but not at that 
size (and most "native" OS X applications indeed use a range of smaller sizes, 
depending on role.

It does have introduce a number of regressions, which the current patch aims to 
address. The most visible and problematic of these regressions is the loss of 
the Mac-style menu bar and thus of all menu items (actions).

The fix is straightforward : on OS X (and similarly affected platforms?), an 
instance of the native Cocoa platform theme is created through the private API, 
and used as a fallback rather than immediately falling back to the default 
implementations from `QPlatformTheme`. In addition, methods missing from (not 
overridden by) `KdePlatformTheme` are provided on OS X and call the 
corresponding methods from the native theme. It is this change which restores 
the menubar and even the Dock menu functionality.
One minor regression remains but should be easy to fix (elsewhere?): the 
Preferences menu loses its keyboard shortcut (Command-,).

Given the fallback nature of the native platform instance I have preferred to 
print a warning rather than using something like `qFatal`, above all because 
the message printed by qFatal tends to get lost on OS X. I can replace my use 
of `qWarning` with a dialog giving the choice between continuing or exiting the 
application - code that would be called in the menu methods because only there 
is it certain that the application actually needs a menubar.

In line with experience and feedback from the KDE(4)-Mac community I have 
decided to force the use of native dialogs rather than the ones from the 
KdePlatformPlugin.

In addition I set the fallback value for `ShowIconsOnPushButtons` to false in 
line with platform guidelines, and ensure that the autotests are not built as 
app bundles.


Diffs
-

  autotests/CMakeLists.txt 7c2129c 
  src/kstyle/kstyle.cpp 6ba5d51 
  src/platformtheme/kdeplatformtheme.h 97d09df 
  src/platformtheme/kdeplatformtheme.cpp 80dbcb7 
  src/platformtheme/khintssettings.cpp 8adf6c5 

Diff: https://git.reviewboard.kde.org/r/126198/diff/


Testing
---

On Mac OS X with Qt 5.5.1, KF5 frameworks 5.16.0 and QtCurve git/head.

I have not verified to what extent my use of a private `QGuiApplication` API 
links builds to a specific Qt version (I consider that nothing shocking and a 
minor price to pay).
>>> Do I need to add some glue to the CMake file so that it will warn if the 
>>> private headerfiles are not available? Apparently no changes were required 
>>> to find them.


File Attachments (updated)


purely native OS X theme
  

Re: Review Request 126198: [OS X] adaptations for the KdePlatformTheme (and autotests)

[René J . V . Bertin]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/126198/
---

(Updated Nov. 30, 2015, 4:51 p.m.)


Review request for KDE Software on Mac OS X, KDE Frameworks and Valorie 
Zimmerman.


Changes
---

Hopefully all 4 snaps will upload, and in the correct order...

I didn't want to include example screenshots at first because atm. the only 
Qt5-based application I have have with a more complex UI is Qt Creator, and 
that one isn't the most representive (it uses an internal theme).

Also, please disregard my actual font of choice, focus on font size and style 
instead.

The 1st snap should show give an indication of what the native theming looks 
like on OS X (and Qt Creator actually looks a bit better than many other Qt5 
applications). You get a single large font (with a much smaller size where 
apparently a bold typeface should be used); not just in the dialog but also in 
the bottom toolbar where it's a more constant waste of space. You also get a 
tabbing style that really shows its age. It's still used by some OS X 
applications, but I notice it less and less.

The 2nd snap shows the effect of using `-style kde` with the native plugin: 
widget drawing routines are taken from the KDE theme (here QtCurve), but the 
fonts are mostly unchanged.

The 3rd snap is the result that can be obtained with the patches proposed here, 
using again QtCurve configured to fit in as well as possible with the native 
look. Except for the typeface of course (though that's not entirely true on my 
own desktop :)).

Lastly, a 4th snap is intended for comparison with the 1st: it shows how the 
same dialog (from the same Qt version) looks on Linux when not using any KDE 
theming.


Repository: frameworkintegration


Description
---

The KdePlatformTheme plugin can be enabled on OS X with a minimal patch to Qt5; 
all that is required is to include the `qgenericunixservices` and 
`qgenericunixthemes` components in the build, and to append `"kde"` to the list 
returned by `QCocoaIntegration::themeNames()` for instance under the condition 
that `KDE_SESSION_VERSION` is set to a suitable value in the environment.

This will allow KF5 and Qt5 applications to use the theme selected through KDE 
if FrameworkIntegration is installed and KDE_SESSION_VERSION is set, which 
seems like a sufficiently specific set of conditions that is easy to avoid by 
users who prefer to use the Mac native theme.

While requestion the KDE theme is also possible through `-style kde` or `env 
QT_STYLE_OVERRIDE=kde` the use of the KdePlatformTheme plugin appears to be the 
only way to get the full theme, including the font and colour selection. In my 
opinion it is above all the font customisation which is a very welcome feature 
for Qt/Mac; by default Qt applications use the default system font (Lucida 
Grande 13pt or even 14pt) throughout. This is a good UI font, but not at that 
size (and most "native" OS X applications indeed use a range of smaller sizes, 
depending on role.

It does have introduce a number of regressions, which the current patch aims to 
address. The most visible and problematic of these regressions is the loss of 
the Mac-style menu bar and thus of all menu items (actions).

The fix is straightforward : on OS X (and similarly affected platforms?), an 
instance of the native Cocoa platform theme is created through the private API, 
and used as a fallback rather than immediately falling back to the default 
implementations from `QPlatformTheme`. In addition, methods missing from (not 
overridden by) `KdePlatformTheme` are provided on OS X and call the 
corresponding methods from the native theme. It is this change which restores 
the menubar and even the Dock menu functionality.
One minor regression remains but should be easy to fix (elsewhere?): the 
Preferences menu loses its keyboard shortcut (Command-,).

Given the fallback nature of the native platform instance I have preferred to 
print a warning rather than using something like `qFatal`, above all because 
the message printed by qFatal tends to get lost on OS X. I can replace my use 
of `qWarning` with a dialog giving the choice between continuing or exiting the 
application - code that would be called in the menu methods because only there 
is it certain that the application actually needs a menubar.

In line with experience and feedback from the KDE(4)-Mac community I have 
decided to force the use of native dialogs rather than the ones from the 
KdePlatformPlugin.

In addition I set the fallback value for `ShowIconsOnPushButtons` to false in 
line with platform guidelines, and ensure that the autotests are not built as 
app bundles.


Diffs
-

  autotests/CMakeLists.txt 7c2129c 
  src/kstyle/kstyle.cpp 6ba5d51 
  src/platformtheme/kdeplatformtheme.h 97d09df 
  src/platformtheme/kdeplatformtheme.cpp 80dbcb7 
  

Re: Review Request 126198: [OS X] adaptations for the KdePlatformTheme (and autotests)

[René J . V . Bertin]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/126198/#review88970
---



File Attachment: using the KDEPlatformTheme - Screen Shot 2015-11-30 at 
15.43.31.png


NB: I'm *not* pushing this as a means to improve the looks of pure Qt 
applications. That's just an added benefit - one I appreciate, but not more 
than that.


- René J.V. Bertin


On Nov. 30, 2015, 4:51 p.m., René J.V. Bertin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126198/
> ---
> 
> (Updated Nov. 30, 2015, 4:51 p.m.)
> 
> 
> Review request for KDE Software on Mac OS X, KDE Frameworks and Valorie 
> Zimmerman.
> 
> 
> Repository: frameworkintegration
> 
> 
> Description
> ---
> 
> The KdePlatformTheme plugin can be enabled on OS X with a minimal patch to 
> Qt5; all that is required is to include the `qgenericunixservices` and 
> `qgenericunixthemes` components in the build, and to append `"kde"` to the 
> list returned by `QCocoaIntegration::themeNames()` for instance under the 
> condition that `KDE_SESSION_VERSION` is set to a suitable value in the 
> environment.
> 
> This will allow KF5 and Qt5 applications to use the theme selected through 
> KDE if FrameworkIntegration is installed and KDE_SESSION_VERSION is set, 
> which seems like a sufficiently specific set of conditions that is easy to 
> avoid by users who prefer to use the Mac native theme.
> 
> While requestion the KDE theme is also possible through `-style kde` or `env 
> QT_STYLE_OVERRIDE=kde` the use of the KdePlatformTheme plugin appears to be 
> the only way to get the full theme, including the font and colour selection. 
> In my opinion it is above all the font customisation which is a very welcome 
> feature for Qt/Mac; by default Qt applications use the default system font 
> (Lucida Grande 13pt or even 14pt) throughout. This is a good UI font, but not 
> at that size (and most "native" OS X applications indeed use a range of 
> smaller sizes, depending on role.
> 
> It does have introduce a number of regressions, which the current patch aims 
> to address. The most visible and problematic of these regressions is the loss 
> of the Mac-style menu bar and thus of all menu items (actions).
> 
> The fix is straightforward : on OS X (and similarly affected platforms?), an 
> instance of the native Cocoa platform theme is created through the private 
> API, and used as a fallback rather than immediately falling back to the 
> default implementations from `QPlatformTheme`. In addition, methods missing 
> from (not overridden by) `KdePlatformTheme` are provided on OS X and call the 
> corresponding methods from the native theme. It is this change which restores 
> the menubar and even the Dock menu functionality.
> One minor regression remains but should be easy to fix (elsewhere?): the 
> Preferences menu loses its keyboard shortcut (Command-,).
> 
> Given the fallback nature of the native platform instance I have preferred to 
> print a warning rather than using something like `qFatal`, above all because 
> the message printed by qFatal tends to get lost on OS X. I can replace my use 
> of `qWarning` with a dialog giving the choice between continuing or exiting 
> the application - code that would be called in the menu methods because only 
> there is it certain that the application actually needs a menubar.
> 
> In line with experience and feedback from the KDE(4)-Mac community I have 
> decided to force the use of native dialogs rather than the ones from the 
> KdePlatformPlugin.
> 
> In addition I set the fallback value for `ShowIconsOnPushButtons` to false in 
> line with platform guidelines, and ensure that the autotests are not built as 
> app bundles.
> 
> 
> Diffs
> -
> 
>   autotests/CMakeLists.txt 7c2129c 
>   src/kstyle/kstyle.cpp 6ba5d51 
>   src/platformtheme/kdeplatformtheme.h 97d09df 
>   src/platformtheme/kdeplatformtheme.cpp 80dbcb7 
>   src/platformtheme/khintssettings.cpp 8adf6c5 
> 
> Diff: https://git.reviewboard.kde.org/r/126198/diff/
> 
> 
> Testing
> ---
> 
> On Mac OS X with Qt 5.5.1, KF5 frameworks 5.16.0 and QtCurve git/head.
> 
> I have not verified to what extent my use of a private `QGuiApplication` API 
> links builds to a specific Qt version (I consider that nothing shocking and a 
> minor price to pay).
> >>> Do I need to add some glue to the CMake file so that it will warn if the 
> >>> private headerfiles are not available? Apparently no changes were 
> >>> required to find them.
> 
> 
> File Attachments
> 
> 
> purely native OS X theme
>   
> 

Re: Review Request 126198: [OS X] adaptations for the KdePlatformTheme (and autotests)

[Martin Gräßlin]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/126198/#review88960
---


I'm strictly against OSX specific changes in framework integration. In my 
opinion framework integration should only be about Plasma. I'll start a new 
thread about it.

- Martin Gräßlin


On Nov. 29, 2015, 8:53 p.m., René J.V. Bertin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126198/
> ---
> 
> (Updated Nov. 29, 2015, 8:53 p.m.)
> 
> 
> Review request for KDE Software on Mac OS X and KDE Frameworks.
> 
> 
> Repository: frameworkintegration
> 
> 
> Description
> ---
> 
> The KdePlatformTheme plugin can be enabled on OS X with a minimal patch to 
> Qt5; all that is required is to include the `qgenericunixservices` and 
> `qgenericunixthemes` components in the build, and to append `"kde"` to the 
> list returned by `QCocoaIntegration::themeNames()` for instance under the 
> condition that `KDE_SESSION_VERSION` is set to a suitable value in the 
> environment.
> 
> This will allow KF5 and Qt5 applications to use the theme selected through 
> KDE if FrameworkIntegration is installed and KDE_SESSION_VERSION is set, 
> which seems like a sufficiently specific set of conditions that is easy to 
> avoid by users who prefer to use the Mac native theme.
> 
> While requestion the KDE theme is also possible through `-style kde` or `env 
> QT_STYLE_OVERRIDE=kde` the use of the KdePlatformTheme plugin appears to be 
> the only way to get the full theme, including the font and colour selection. 
> In my opinion it is above all the font customisation which is a very welcome 
> feature for Qt/Mac; by default Qt applications use the default system font 
> (Lucida Grande 13pt or even 14pt) throughout. This is a good UI font, but not 
> at that size (and most "native" OS X applications indeed use a range of 
> smaller sizes, depending on role.
> 
> It does have introduce a number of regressions, which the current patch aims 
> to address. The most visible and problematic of these regressions is the loss 
> of the Mac-style menu bar and thus of all menu items (actions).
> 
> The fix is straightforward : on OS X (and similarly affected platforms?), an 
> instance of the native Cocoa platform theme is created through the private 
> API, and used as a fallback rather than immediately falling back to the 
> default implementations from `QPlatformTheme`. In addition, methods missing 
> from (not overridden by) `KdePlatformTheme` are provided on OS X and call the 
> corresponding methods from the native theme. It is this change which restores 
> the menubar and even the Dock menu functionality.
> One minor regression remains but should be easy to fix (elsewhere?): the 
> Preferences menu loses its keyboard shortcut (Command-,).
> 
> Given the fallback nature of the native platform instance I have preferred to 
> print a warning rather than using something like `qFatal`, above all because 
> the message printed by qFatal tends to get lost on OS X. I can replace my use 
> of `qWarning` with a dialog giving the choice between continuing or exiting 
> the application - code that would be called in the menu methods because only 
> there is it certain that the application actually needs a menubar.
> 
> In line with experience and feedback from the KDE(4)-Mac community I have 
> decided to force the use of native dialogs rather than the ones from the 
> KdePlatformPlugin.
> 
> In addition I set the fallback value for `ShowIconsOnPushButtons` to false in 
> line with platform guidelines, and ensure that the autotests are not built as 
> app bundles.
> 
> 
> Diffs
> -
> 
>   autotests/CMakeLists.txt 7c2129c 
>   src/kstyle/kstyle.cpp 6ba5d51 
>   src/platformtheme/kdeplatformtheme.h 97d09df 
>   src/platformtheme/kdeplatformtheme.cpp 80dbcb7 
>   src/platformtheme/khintssettings.cpp 8adf6c5 
> 
> Diff: https://git.reviewboard.kde.org/r/126198/diff/
> 
> 
> Testing
> ---
> 
> On Mac OS X with Qt 5.5.1, KF5 frameworks 5.16.0 and QtCurve git/head.
> 
> I have not verified to what extent my use of a private `QGuiApplication` API 
> links builds to a specific Qt version (I consider that nothing shocking and a 
> minor price to pay).
> >>> Do I need to add some glue to the CMake file so that it will warn if the 
> >>> private headerfiles are not available? Apparently no changes were 
> >>> required to find them.
> 
> 
> Thanks,
> 
> René J.V. Bertin
> 
>

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 126198: [OS X] adaptations for the KdePlatformTheme (and autotests)

[René J . V . Bertin]

> On Nov. 30, 2015, 12:42 p.m., Martin Gräßlin wrote:
> > I'm strictly against OSX specific changes in framework integration. In my 
> > opinion framework integration should only be about Plasma. I'll start a new 
> > thread about it.

I was afraid you'd be saying that.
 
I can only hope that this will not become the consensus.

I could counter that even if *this* particular framework is going to be 
re-assigned to Plasma session there is still the possibility to provide a 
comparable, possibly stripped-down framework which would be about integration 
(and improving the KF5 experience and that of its customisation possibilities) 
on other platforms. Seems like a bit of a waste of resources though, given how 
little there is to strip from the code currently.

NB: I can rewrite the patch so that different files will be used on OS X rather 
than using #ifdefs.

Fellow KDE-Mac users, I'm doing this for the sake of our community so don't let 
me be a single voice in the desert.


- René J.V.


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/126198/#review88960
---


On Nov. 29, 2015, 8:53 p.m., René J.V. Bertin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126198/
> ---
> 
> (Updated Nov. 29, 2015, 8:53 p.m.)
> 
> 
> Review request for KDE Software on Mac OS X and KDE Frameworks.
> 
> 
> Repository: frameworkintegration
> 
> 
> Description
> ---
> 
> The KdePlatformTheme plugin can be enabled on OS X with a minimal patch to 
> Qt5; all that is required is to include the `qgenericunixservices` and 
> `qgenericunixthemes` components in the build, and to append `"kde"` to the 
> list returned by `QCocoaIntegration::themeNames()` for instance under the 
> condition that `KDE_SESSION_VERSION` is set to a suitable value in the 
> environment.
> 
> This will allow KF5 and Qt5 applications to use the theme selected through 
> KDE if FrameworkIntegration is installed and KDE_SESSION_VERSION is set, 
> which seems like a sufficiently specific set of conditions that is easy to 
> avoid by users who prefer to use the Mac native theme.
> 
> While requestion the KDE theme is also possible through `-style kde` or `env 
> QT_STYLE_OVERRIDE=kde` the use of the KdePlatformTheme plugin appears to be 
> the only way to get the full theme, including the font and colour selection. 
> In my opinion it is above all the font customisation which is a very welcome 
> feature for Qt/Mac; by default Qt applications use the default system font 
> (Lucida Grande 13pt or even 14pt) throughout. This is a good UI font, but not 
> at that size (and most "native" OS X applications indeed use a range of 
> smaller sizes, depending on role.
> 
> It does have introduce a number of regressions, which the current patch aims 
> to address. The most visible and problematic of these regressions is the loss 
> of the Mac-style menu bar and thus of all menu items (actions).
> 
> The fix is straightforward : on OS X (and similarly affected platforms?), an 
> instance of the native Cocoa platform theme is created through the private 
> API, and used as a fallback rather than immediately falling back to the 
> default implementations from `QPlatformTheme`. In addition, methods missing 
> from (not overridden by) `KdePlatformTheme` are provided on OS X and call the 
> corresponding methods from the native theme. It is this change which restores 
> the menubar and even the Dock menu functionality.
> One minor regression remains but should be easy to fix (elsewhere?): the 
> Preferences menu loses its keyboard shortcut (Command-,).
> 
> Given the fallback nature of the native platform instance I have preferred to 
> print a warning rather than using something like `qFatal`, above all because 
> the message printed by qFatal tends to get lost on OS X. I can replace my use 
> of `qWarning` with a dialog giving the choice between continuing or exiting 
> the application - code that would be called in the menu methods because only 
> there is it certain that the application actually needs a menubar.
> 
> In line with experience and feedback from the KDE(4)-Mac community I have 
> decided to force the use of native dialogs rather than the ones from the 
> KdePlatformPlugin.
> 
> In addition I set the fallback value for `ShowIconsOnPushButtons` to false in 
> line with platform guidelines, and ensure that the autotests are not built as 
> app bundles.
> 
> 
> Diffs
> -
> 
>   autotests/CMakeLists.txt 7c2129c 
>   src/kstyle/kstyle.cpp 6ba5d51 
>   src/platformtheme/kdeplatformtheme.h 97d09df 
>   src/platformtheme/kdeplatformtheme.cpp 80dbcb7 
>   src/platformtheme/khintssettings.cpp 8adf6c5 
> 
> Diff: 

Re: Review Request 126198: [OS X] adaptations for the KdePlatformTheme (and autotests)

[René J . V . Bertin]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/126198/
---

(Updated Nov. 29, 2015, 8:53 p.m.)


Review request for KDE Software on Mac OS X and KDE Frameworks.


Changes
---

This revision fixes the menu item shortcut issue by always returning the 
keybindings from the native platform theme, and by cutting down on the number 
of themeHints provided by the KDEPlatformPlugin.

A lesson learned by Microsoft long ago: menu shortcuts shouldn't be translated 
nor set to key combinations from foreign systems. :)


Repository: frameworkintegration


Description
---

The KdePlatformTheme plugin can be enabled on OS X with a minimal patch to Qt5; 
all that is required is to include the `qgenericunixservices` and 
`qgenericunixthemes` components in the build, and to append `"kde"` to the list 
returned by `QCocoaIntegration::themeNames()` for instance under the condition 
that `KDE_SESSION_VERSION` is set to a suitable value in the environment.

This will allow KF5 and Qt5 applications to use the theme selected through KDE 
if FrameworkIntegration is installed and KDE_SESSION_VERSION is set, which 
seems like a sufficiently specific set of conditions that is easy to avoid by 
users who prefer to use the Mac native theme.

While requestion the KDE theme is also possible through `-style kde` or `env 
QT_STYLE_OVERRIDE=kde` the use of the KdePlatformTheme plugin appears to be the 
only way to get the full theme, including the font and colour selection. In my 
opinion it is above all the font customisation which is a very welcome feature 
for Qt/Mac; by default Qt applications use the default system font (Lucida 
Grande 13pt or even 14pt) throughout. This is a good UI font, but not at that 
size (and most "native" OS X applications indeed use a range of smaller sizes, 
depending on role.

It does have introduce a number of regressions, which the current patch aims to 
address. The most visible and problematic of these regressions is the loss of 
the Mac-style menu bar and thus of all menu items (actions).

The fix is straightforward : on OS X (and similarly affected platforms?), an 
instance of the native Cocoa platform theme is created through the private API, 
and used as a fallback rather than immediately falling back to the default 
implementations from `QPlatformTheme`. In addition, methods missing from (not 
overridden by) `KdePlatformTheme` are provided on OS X and call the 
corresponding methods from the native theme. It is this change which restores 
the menubar and even the Dock menu functionality.
One minor regression remains but should be easy to fix (elsewhere?): the 
Preferences menu loses its keyboard shortcut (Command-,).

Given the fallback nature of the native platform instance I have preferred to 
print a warning rather than using something like `qFatal`, above all because 
the message printed by qFatal tends to get lost on OS X. I can replace my use 
of `qWarning` with a dialog giving the choice between continuing or exiting the 
application - code that would be called in the menu methods because only there 
is it certain that the application actually needs a menubar.

In line with experience and feedback from the KDE(4)-Mac community I have 
decided to force the use of native dialogs rather than the ones from the 
KdePlatformPlugin.

In addition I set the fallback value for `ShowIconsOnPushButtons` to false in 
line with platform guidelines, and ensure that the autotests are not built as 
app bundles.


Diffs (updated)
-

  autotests/CMakeLists.txt 7c2129c 
  src/kstyle/kstyle.cpp 6ba5d51 
  src/platformtheme/kdeplatformtheme.h 97d09df 
  src/platformtheme/kdeplatformtheme.cpp 80dbcb7 
  src/platformtheme/khintssettings.cpp 8adf6c5 

Diff: https://git.reviewboard.kde.org/r/126198/diff/


Testing
---

On Mac OS X with Qt 5.5.1, KF5 frameworks 5.16.0 and QtCurve git/head.

I have not verified to what extent my use of a private `QGuiApplication` API 
links builds to a specific Qt version (I consider that nothing shocking and a 
minor price to pay).
>>> Do I need to add some glue to the CMake file so that it will warn if the 
>>> private headerfiles are not available? Apparently no changes were required 
>>> to find them.


Thanks,

René J.V. Bertin

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel