> On June 22, 2013, 12:58 p.m., Thomas Lübking wrote: > > kdeui/util/kglobalsettings.cpp, line 308 > > <http://git.reviewboard.kde.org/r/111171/diff/1/?file=165092#file165092line308> > > > > 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. > > Aleix Pol Gonzalez wrote: > Are you sure these are being used? because I searched for uses of this > API and I didn't see such uses. > > Thomas Lübking wrote: > 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. > > Martin Gräßlin wrote: > 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. > > Thomas Lübking wrote: > 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 ;-) > > Martin Gräßlin wrote: > wtf?!? I hate magic functionality > > Thomas Lübking wrote: > 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. > > Aleix Pol Gonzalez wrote: > 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. > > Thomas Lübking wrote: > 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¢ > > Aaron J. Seigo wrote: > "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. > > David Faure wrote: > 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. > > Kevin Ottens wrote: > 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. > > Aurélien Gâteau wrote: > 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. > > > > Thomas Lübking wrote: > > https://techbase.kde.org/KDE_System_Administration/Kiosk/Introduction#KDE_Action_Restrictions > > Aurélien Gâteau wrote: > 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(). > > Thomas Lübking wrote: > 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. > > Kevin Ottens wrote: > Yes, please move that debate somewhere else... hijacking that review for > that is just making things painful at that point. > > Now bottom line (and I'm repeating myself to give a direction to that > review again): > Please move those color settings to be kwin only. Thanks.
Agreed, let's move this to kde-core-devel. - Aurélien ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/111171/#review34861 ----------------------------------------------------------- On June 22, 2013, 12:25 p.m., Àlex Fiestas wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://git.reviewboard.kde.org/r/111171/ > ----------------------------------------------------------- > > (Updated June 22, 2013, 12:25 p.m.) > > > Review request for KDE Frameworks and kdelibs. > > > Description > ------- > > Deprecate: inactiveTitleColor, inactiveTextColor, activeTitleColor, > activeTextColor in favor of KColorScheme and replace the implementation of > those methods with it. > > > Diffs > ----- > > kdeui/util/kglobalsettings.h 4b77ed5 > kdeui/util/kglobalsettings.cpp 3e60632 > khtml/misc/helper.cpp dccb9bf > > Diff: http://git.reviewboard.kde.org/r/111171/diff/ > > > Testing > ------- > > 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. > > > Thanks, > > Àlex Fiestas > >
