[prev in list] [next in list] [prev in thread] [next in thread] 

List:       kde-core-devel
Subject:    Re: Fwd: Re: PATCH - Mega style patch
From:       Neil Stevens <neil () qualityassistant ! com>
Date:       2002-03-01 4:54:43
[Download RAW message or body]

On Thursday February 28, 2002 08:52, Fredrik Höglund wrote:
> ----------  Forwarded Message  ----------
>
> Subject: Re: PATCH - Mega style patch
> Date: Thu, 28 Feb 2002 23:32:49 -0500
> From: Maks Orlovich <mo002j@mail.rochester.edu>
> To: Fredrik H?glund <fredrik@kde.org>
>
> Hi, can you please forward this to core-devel? This is quite important,
> and I don't have write access..
>
> -Thanks, Sad Eagle
>
> > Right now there's a big gaping problem in the Style handling - If a
> > style uses KDE, it gets applied to Qt anyway, and can take down all a
> > user's Qt apps, most notably the very qtconfig that would be needed to
> > fix it.
>
> First of all, it needs to be shown that this is truly necessary, IMHO,
> particularly since it touches KApplication.

The first version of Liquid for KDE 3 used kapp, which made Qt apps fail, 
including qtconfig which would be needed to repair it.  Liquid was since 
made to work with Qt-only apps, but the next style author may not care, 
which will create hardship on users of Qt-only apps who want that style.

That's the simple fact.  KDE applies styles to Qt indiscriminately, in a 
careless manner that jeopardizes the very users this system was supposed 
to help - users of Qt apps.

> First, the way you currently do enumeration in kcontrol ignores the fact
> that a style may not list itself in the plugin's keys() method but
> provide .themerc, for example to run only on displays of sufficient
> color depth. Further, if a style like that gets selected, the factory
> method will return 0, so your nice fallback code will chose the default
> -- sane behavior, but not what the user would expect when selecting "The
> new fancy style" in kcontrol.

No it doesn't.  It only uses keys() for styles that *don't* have themerc 
files.  It checks themerc files *first*.

> Also, why are you doing tolower() in all the style plugins, instead of
> doing it in one spot in the loader?

I just copied over from light what seemed to be the correct behavior, 
which seemed especially since Highcolor was using different case at 
different times.

> Not returning void* from kdeStyleFactory would be a nice touch too,
> IMHO. What this returns should always be an instance of something that
> inherits off QStyle.

Easy to fix.

>
> > * kstyles are modified to make their Qt enumeration name consistent
> > with their .themerc name
> > * themerc files have the .la file added as a new key, WidgetStyleLib
>
> This will force all pixmap themes to be modified to work with KDE3;
> again, in fairness they need to be modified to work with "old" kcm_style
> anyway, and I need to do some work still for it to be completely
> backwards compatible, but I would appreciate if that were not rendered
> impossible. Plus, someone will hurt me if I say that the .themerc's for
> their favorite pixmap themes need to be modified yet again ;-)

Someone whose Qt apps are broken because a KDE style author who didn't 
care about working with Qt apps, and who finds out that a way of avoiding 
this is rejected, will want to hurt you as well.  Or just be left with a 
distaste for KDE, that finger pointing (blaming the style author) would 
take precedence over robustness (avoiding KDE-only styles being applied to 
Qt in the first place).

> Also, as someone who did a lot to various pixmap themes's themerc's at
> various stages of development of KThemeStyle's KDE3 port, I can tell you
> that having information on styles in multiple places is a potential
> headache...

You already had two places before this patch - keys() and themerc.  After 
this patch there are two places - keys() and themerc.  The number isn't 
increased, so that's is no reason to ignore this fix.


-- 
Neil Stevens
neil@qualityassistant.com

Don't think of a bug as a problem.  Think of it as a call to action.
[prev in list] [next in list] [prev in thread] [next in thread]