Review Request 111171: Deprecate (in)active(Title/Text)Color in favor of KColorScheme

[Kevin Ottens]

> On July 1, 2013, 2:39 p.m., Kevin Ottens wrote:
> > kdeui/util/kglobalsettings.h, line 265
> > <http://git.reviewboard.kde.org/r/111171/diff/2/?file=166657#file166657line265>
> >
> >     I think it's fine. lxr.kde.org seem to indicate that those colors got abused at some places anyway, and what's proposed here seems like a proper replacement to me (except if you're kwin of course, but I think the people behind it know that by now).
> 
> Thomas Lübking wrote:
>     "@deprecated since 5.0, This used to export the color of the titlebar. If you wanted to query the color of the window background, use KColorScheme(QPalette::Active, KColorScheme::Window).foreground(KColorScheme::ActiveText).color(), otherwise please read the setting from <where it's moved> directly"?

<where it's moved> would be kwin

Yes, sounds good to me.


> On July 1, 2013, 2:39 p.m., Kevin Ottens wrote:
> > khtml/misc/helper.cpp, line 160
> > <http://git.reviewboard.kde.org/r/111171/diff/2/?file=166658#file166658line160>
> >
> >     Why unrelated?
> >     
> >     It's removing calls to the newly deprecated API from KHTML (one of the abusers from my previous comment). Could be made in a separate commit indeed, but for review purpose it seems relevant to the topic for me.
> 
> Thomas Lübking wrote:
>     The function so far handed the color of the titlebar, now it hands the color of the window.
>     
>     a) this has been wrong so far (as you suggest):
>        then this fixes an unrelated present bug - i personally don't mid too much, but it's still an unrelated bug ;P
>     b) this has been correct so far:
>        then this introduces a behavioral change, implying titlebar color == window color, which is either intentionally unrelated (only relation being the opportunity) or unintentionally, ie. the change in reaction to the deprecation is wrong and accidentally alters behavior.
>     
>     To recap my worries and position on this topic:
>        If there shall be a global alignment of "titlebar color == window background color" (with the freedom of single decorations to deviate, of course) I want that decision explicitly *taken* and not somehow implied - to avoid any sequel to that oxygen/ozone kindergarten session.

I would be (a) in this case. So yes, would have been better as a separate commit, but that's no big deal IMO.


- Kevin


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/111171/#review35358
-----------------------------------------------------------


On June 28, 2013, 5:10 p.m., Àlex Fiestas wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/111171/
> -----------------------------------------------------------
> 
> (Updated June 28, 2013, 5:10 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 
>   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
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-core-devel/attachments/20130702/f3db0dcf/attachment.htm>