<html>
<body>
<div style="font-family: Verdana, Arial, Helvetica, Sans-Serif;">
<table bgcolor="#f9f3c9" width="100%" cellpadding="8" style="border: 1px #c9c399 solid;">
<tr>
<td>
This is an automatically generated e-mail. To reply, visit:
<a href="http://git.reviewboard.kde.org/r/111171/">http://git.reviewboard.kde.org/r/111171/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On June 22nd, 2013, 10:58 a.m. UTC, <b>Thomas Lübking</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<table width="100%" border="0" bgcolor="white" style="border: 1px solid #C0C0C0; border-collapse: collapse; margin: 2px padding: 2px;">
<thead>
<tr>
<th colspan="4" bgcolor="#F0F0F0" style="border-bottom: 1px solid #C0C0C0; font-size: 9pt; padding: 4px 8px; text-align: left;">
<a href="http://git.reviewboard.kde.org/r/111171/diff/1/?file=165092#file165092line308" style="color: black; font-weight: bold; text-decoration: underline;">kdeui/util/kglobalsettings.cpp</a>
<span style="font-weight: normal;">
(Diff revision 1)
</span>
</th>
</tr>
</thead>
<tbody style="background-color: #e4d9cb; padding: 4px 8px; text-align: center;">
<tr>
<td colspan="4"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">KGlobalSettings::Completion KGlobalSettings::completionMode()</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#ebb1ba" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">308</font></th>
<td bgcolor="#ffc5ce" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="k">return</span> <span class="n">g</span><span class="p">.</span><span class="n">readEntry</span><span class="p">(</span> <span class="s">"activeBackground"</span><span class="p">,</span> <span class="n">QColor</span><span class="p">(</span><span class="mi">48</span><span class="p">,</span><span class="mi">174</span><span class="p">,</span><span class="mi">232</span><span class="p">));</span></pre></td>
<th bgcolor="#ebb1ba" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2"></font></th>
<td bgcolor="#ffc5ce" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
</tr>
</tbody>
</table>
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">These are the colors for the window titlbar (with ambiguous function names, though), the default for activeTitleColor used to be #30AEE8 - that is blue!
Now you want to return the active window background (ie. usually gray) - iow: enforce the color alignment that once caused the former KWin maintainer to for the oxygen deco into the new default ozone?
I mean, decorations can still keep their internal color settings (what was somehow required because the titlebar color was initially happily removed from the color kcm - instead one could define the button color in listviews...), but the implication of this change is that the titlbar can no more be individually configured and *has* to align to the window background.
-> The RR should clearly state that central color configuration for titlebars will ultimately be unsupported.
Until then, the change can imo not go in before KF5 anyway, because it's a very visible behavioral change for users of Laptop, Plastik, BII and some legacy decorations (you still /can/ compile Quartz and Keramik and what they all were called) and pot. some distro specific decorations.</pre>
</blockquote>
<p>On June 22nd, 2013, 11:25 a.m. UTC, <b>Aleix Pol Gonzalez</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Are you sure these are being used? because I searched for uses of this API and I didn't see such uses.</pre>
</blockquote>
<p>On June 22nd, 2013, 11:41 a.m. UTC, <b>Thomas Lübking</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">KWin reads the setting values by hand in libkdecorations/kdecoration_p.cpp - so actually I was wrong in that aspect: the change in KGlobalSettings will not have a visible impact here - no guarantee for 3rd party decos beyond bespin (esp. on custom config dialogs)
Not sure whether this makes the change better, since now KWin will no longer reflect what KGlobalSettings reports.
I'm not sure how important it is to export those values via KGlobalSettings (i guess they existed to use the colors on MDI windows from KStyle), but even while deprecated, their behavior should not change.
And unless the conclusion is that titlebars shall align to the window content color, the advice should also not be to query that instead. It's better to state "information not avialable" than to give a false information.</pre>
</blockquote>
<p>On June 22nd, 2013, 11:45 a.m. UTC, <b>Martin Gräßlin</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">It's worse - KWin does not just read the values by itself, it reads them from kwinrc instead of kdeglobals. So whatever is set in KGlobalSettings: KWin ignores it.</pre>
</blockquote>
<p>On June 22nd, 2013, 12:47 p.m. UTC, <b>Thomas Lübking</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">No ;-)
Welcome to the magics of KConfig.
I for one have no [WM] entry in kwinrc, it's only in kdeglobal - try grepping it =)
However, KConfig resolves to kdeglobal for *every* rc unless explicitly told do do not, so <advert>
kconfig kdeglobal/WM list
and
kconfig kwinrc/WM list
</advert>
will print the same value (read from kdeglobal) unless you explicitly add a configuration to kwinrc (what's afaics not done anywhere - hopefully ;-)</pre>
</blockquote>
<p>On June 22nd, 2013, 1:02 p.m. UTC, <b>Martin Gräßlin</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">wtf?!? I hate magic functionality</pre>
</blockquote>
<p>On June 22nd, 2013, 1:12 p.m. UTC, <b>Thomas Lübking</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Well, yes. The "magic" function here is the open flag:
http://api.kde.org/4.10-api/kdelibs-apidocs/kdecore/html/classKConfig.html#ad1f23964bbf8c11449e92a2596d15f7e
One can either omit cascading (system wide files) or globals (kdeglobals)
Since the color and some WM settings were/are attractive from many points, it was/is reasonable to store them globally.</pre>
</blockquote>
<p>On June 22nd, 2013, 2:27 p.m. UTC, <b>Aleix Pol Gonzalez</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">In any case, this seems broken to me.
If you're really serious about this, we probably should just deprecate the function and advise users to just read the file directly if they want to access this data and move on.</pre>
</blockquote>
<p>On June 22nd, 2013, 2:41 p.m. UTC, <b>Thomas Lübking</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">No, it's not broken at all - normal KConfig behavior, ever has been.
If this shall be deprecated (can't say - i don't need them and don't care) your conclusion is however right:
the current behavior shall be preserved and the clients be advised to query the setting directly (as kwin does atm) for KF5.
The "serious" part is that you were about to alter functionality in even frozen kdelibs and esp. imply a wider reaching change: fixed "titlebar == window background" assumption, ie. no titlebar colors - which, given the little war on oxygen back then, probably needs to be openly discussed before.
Personally, i'd be fine moving the setting to kwin only - as MDI is a bug to begin with and the setting unlikely required by applications then.
I simply didn't want KWin to receive bugreports "plastik: colors no more supported" (what, as now figured, would however only happen by withdrawing the color kcm setting on top of mentioned implication) and i *seriously* do not want to see such war as on ozone ./. oxygen ever again (so the idea to deprecate those color config triggered a button ;-)
/my2¢</pre>
</blockquote>
<p>On June 24th, 2013, 7:58 a.m. UTC, <b>Aaron J. Seigo</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">"I hate magic functionality"
... any sufficiently advanced technology *cough*
As noted, this is documented behaviour and it serves a very real and very useful purpose: to allow global settings to be transparently available to (yet still be overridden by!) each application. It's a very simple and elegant way to allow for global behaviours to be configured without requiring such code in every application.</pre>
</blockquote>
<p>On June 24th, 2013, 8:15 a.m. UTC, <b>David Faure</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Aaron is completely right. But of course the case of WM colors is a really bad one for kdeglobals. Having them there pollutes the KConfig (= eats more memory) in each application, which typically doesn't care for these settings.</pre>
</blockquote>
<p>On June 24th, 2013, 1:12 p.m. UTC, <b>Kevin Ottens</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Exactly, having KConfig also look in kdeglobals is definitely a magic we want to keep. Now in the case of those particular colors it's likely a kind of abuse, I'd like to see them moved to be kwin only. It looks like the best way forward.</pre>
</blockquote>
<p>On June 24th, 2013, 2:51 p.m. UTC, <b>Aurélien Gâteau</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">I see the point of having cross-app configuration files, I assume classes from kdelibs for example can use it to store settings. What I don't understand is in which situation it is useful to have all configuration file inherit from cross-app configuration keys, compared to having cross-app code explicitly call KSharedConfig::openConfig("kdeglobals"). Can someone provide an example?
To me it feels dangerous because application developers may not be aware of this feature and may inherit values for keys they expect to be application specific if they are unlucky to pick a group used in kdeglobals or if a group they used in the past is now used by a component which writes in kdeglobals.
</pre>
</blockquote>
<p>On June 24th, 2013, 6:38 p.m. UTC, <b>Thomas Lübking</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">https://techbase.kde.org/KDE_System_Administration/Kiosk/Introduction#KDE_Action_Restrictions</pre>
</blockquote>
<p>On June 24th, 2013, 8:24 p.m. UTC, <b>Aurélien Gâteau</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Kiosk is actually a good example of why inheriting from kdeglobals can be painful.
I used to work for a company which deployed locked-down KDE systems for dentists. We would create the locked-down configuration by running KDE as an "empty" user, configure the desktop to our liking, then copy the config files from $HOME/.kde to a custom, read-only dir which was inserted in $KDEDIRS. We would of course lock-down the config files from the custom dir by marking their config as immutable. As you know there are 3 ways to lock-down a config file: you can lock the whole file, a whole section or a single key. We usually locked down sections since we felt it was the most convenient way to go. We locked down sections in many files, and at some point we locked down the "General" section of kdeglobals... and wasted quite some time figuring out why many applications would not save all of their settings. You would not believe how common a "General" section is.
Sure it is nice to be able to take advantage of this to lock-down actions at the desktop and/or application level, but that could also be done by first looking at kdeglobals:"KDE Action Restrictions":action/${actionName} and then at ${appname}rc:"KDE Action Restrictions":action/${actionName}. Furthermore, since this code is specific to Kiosk, it makes sense for it to actively look at KConfig::isImmutable() and KConfigGroup::isEntryImmutable().</pre>
</blockquote>
</blockquote>
<pre style="margin-left: 1em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">This went so much OT - sorry that we're abusing your RR, Aleix.
----
Why would the code be specific to kiosk? - You asked for an example, not justification.
It does eg. also allow you to preconfigure the MainWindow settings of all (even yet unwritten) applications - w/o any requirement to immutability, rather explicitly allowing them to be altered per application and account afterwards.
It does as well allow to control all immutability settings from only one file (unless an application doesn't IncludeGlobals when reading its settings)
I'm pretty sure that the [General] section has widespread usage (eg. in gwenview ;-)
But when you draw a broadsword, don't complain about the bloodshed ;-P
(And yes: "general" in "*global*" is a broadsword - or rather even a scythe)
Now, I don't want to defend the behavior of KConfig, but your arguments so far have been:
a) developers not aware of the behavior (ie. using KConfig w/o ever looking at the API doc) and
b) admins using a system w/o understanding its design (for I will not assume that, had you known that the General group from kdeglobals was merged into all General groups of all applications, you had immuted it)
b) Is actually not relevant at all, as even if FullConfig had to be explicitly passed, it would happen nevertheless - and then some applications could have stored their generals and others could not.
Doesn't seem any simpler to track.
About a):
I assume if the API was new, one might make the parameter w/o default - enforcing developers awareness and informed choice.
Nevertheless, using an API by cnp w/o wasting a glimpse at the docu is just wrong.
Happens, yes. I do sometimes, yes. - But it's still wrong.
You certainly have a point on the name collision (so on't use background in [General]...), but usually the local context has precedence, thus the worst thing to happen was that the "vanilla" default value isn't the one you used in the code (until the user stores the desired value once).
Eventually the default actually should have been CascadeOnly (because IncludeGlobals seems mostly interesting to libs only), but "bUseKDEGlobals" has been true by default in even KDE3 - and, unfortunately, we don't live in should-land.
=> I'd suggest to bring this up at an appropriate place w/ an appropriate subject line for KF5.
My opinion? - Changing the behviour has to make the parameter mandatory, ie. break compilation to enforce a developer choice.</pre>
<br />
<p>- Thomas</p>
<br />
<p>On June 22nd, 2013, 10:25 a.m. UTC, Àlex Fiestas wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: url('http://git.reviewboard.kde.org/static/rb/images/review_request_box_top_bg.ab6f3b1072c9.png'); background-position: left top; background-repeat: repeat-x; border: 1px black solid;">
<tr>
<td>
<div>Review request for KDE Frameworks and kdelibs.</div>
<div>By Àlex Fiestas.</div>
<p style="color: grey;"><i>Updated June 22, 2013, 10:25 a.m.</i></p>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Description </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
<tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Deprecate: inactiveTitleColor, inactiveTextColor, activeTitleColor, activeTextColor in favor of KColorScheme and replace the implementation of those methods with it.</pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Testing </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
<tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">I have compared the colors returned by the methods before and after this patch, they are close enough.
Additionally used some apps like filelight with the change, and it seems to work for them as well.</pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Diffs</b> </h1>
<ul style="margin-left: 3em; padding-left: 0;">
<li>kdeui/util/kglobalsettings.h <span style="color: grey">(4b77ed5)</span></li>
<li>kdeui/util/kglobalsettings.cpp <span style="color: grey">(3e60632)</span></li>
<li>khtml/misc/helper.cpp <span style="color: grey">(dccb9bf)</span></li>
</ul>
<p><a href="http://git.reviewboard.kde.org/r/111171/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>