Bug 341940 - colour scheme does not update automatically for some widgets
Summary: colour scheme does not update automatically for some widgets
Status: RESOLVED FIXED
Alias: None
Product: Breeze
Classification: Plasma
Component: QStyle (show other bugs)
Version: unspecified
Platform: Other Linux
: NOR normal
Target Milestone: ---
Assignee: Hugo Pereira Da Costa
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2014-12-16 11:05 UTC by David Edmundson
Modified: 2015-01-24 18:37 UTC (History)
2 users (show)

See Also:
Latest Commit:
Version Fixed In:


Attachments
possible patch to crash reported by Lamarque (1.81 KB, patch)
2015-01-24 17:40 UTC, Hugo Pereira Da Costa
Details

Note You need to log in before you can comment on or make changes to this bug.
Description David Edmundson 2014-12-16 11:05:02 UTC
QTabWidgets and GroupBoxes don't update the palette on the fly when the application changes.

I assume one of the frame helpers is caching a palette. I'm happy to fix it later, but if you happen to know where to look feel free to fix it first :)

(requires latest master frameworks where I fixed normal palette updating from systemsettings)
Comment 1 Hugo Pereira Da Costa 2014-12-16 11:09:30 UTC
yep I know where the issue is
this is because these widget's get an altered palette wrt the QApp one, in order to have the requested slightly lighter background. This is set inside Breeze::Style::polish()
which is not called again after palette change
one might need to move these palette changes to a separate method, that would be called again at palette change. 
But then this also requires to keep track of all GroupBoxes and TabWidgets ...
Comment 2 David Edmundson 2014-12-16 11:45:50 UTC
Yep, that's it.

If I comment out all the setPalettes it works fine.

I think this approach might cause other problems if an application ever tried to set a custom palette on an item.

What might work better, is that QApplication::setApplicationPalette takes a second const* argument which is the name of the widget which should have a different palette applied, and QApplication can do it automajically.

So in the breeze constructor we could do QApplication::setPalette("QTabWidget",  _helper->framePalette( QApplication::palette())   and then we wouldn't need it in the polish.

Then we just need one event filter for the system palette changing to re-update these.
Comment 3 Hugo Pereira Da Costa 2014-12-16 12:19:34 UTC
(In reply to David Edmundson from comment #2)
> Yep, that's it.
> 
> If I comment out all the setPalettes it works fine.
> 
> I think this approach might cause other problems if an application ever
> tried to set a custom palette on an item.
> 
> What might work better, is that QApplication::setApplicationPalette takes a
> second const* argument which is the name of the widget which should have a
> different palette applied, and QApplication can do it automajically.
> 
I did precisely that in the past and it failed, although for a reason which I don't quite remember any more. Can't remember if this was because of this not being honored by KApplication of some story with palette inheritance. Feel free to give it another shot any way if you want.


> So in the breeze constructor we could do
> QApplication::setPalette("QTabWidget",  _helper->framePalette(
> QApplication::palette())   and then we wouldn't need it in the polish.
> 
> Then we just need one event filter for the system palette changing to
> re-update these.
Comment 4 Hugo Pereira Da Costa 2014-12-16 12:20:50 UTC
see 4bfd9c3f9e1ec4333027e1c780dc339e5700ae4d

In fact, the issue might well be that Style::polish( QApplication*) is simply not called (or at least not for some apps)
Comment 5 Hugo Pereira Da Costa 2014-12-16 12:21:49 UTC
(In reply to Hugo Pereira Da Costa from comment #4)
> see 4bfd9c3f9e1ec4333027e1c780dc339e5700ae4d
> 
> In fact, the issue might well be that Style::polish( QApplication*) is
> simply not called (or at least not for some apps)

... as in aaa664a8930aa71088a4936e3d08ea0207335426
Comment 6 Hugo Pereira Da Costa 2014-12-27 17:00:28 UTC
Git commit c2a6c0b087d5d4c65431b4c8284a10976a3f6a02 by Hugo Pereira Da Costa.
Committed on 27/12/2014 at 12:57.
Pushed by hpereiradacosta into branch 'master'.

Keep track of palette change events, to update widgets for which the palettes was altered
accordingly.

M  +1    -0    kstyle/CMakeLists.txt
A  +138  -0    kstyle/breezepalettehelper.cpp     [License: GPL (v2+)]
A  +83   -0    kstyle/breezepalettehelper.h     [License: GPL (v2+)]
M  +6    -39   kstyle/breezestyle.cpp
M  +4    -0    kstyle/breezestyle.h

http://commits.kde.org/breeze/c2a6c0b087d5d4c65431b4c8284a10976a3f6a02
Comment 7 Lamarque V. Souza 2015-01-23 14:28:55 UTC
Hi,

After this change breeze triggers a Qt 4.x assert everytime I try to start any Plasma 4 application:

(gdb) bt
#0  0x0000003bd5638795 in raise () from /lib64/libc.so.6
#1  0x0000003bd5639c18 in abort () from /lib64/libc.so.6
#2  0x000000333ac71154 in qt_message_output(QtMsgType, char const*) () from /usr/lib64/qt4/libQtCore.so.4
#3  0x000000333ac712d9 in qt_message(QtMsgType, char const*, __va_list_tag*) () from /usr/lib64/qt4/libQtCore.so.4
#4  0x000000333ac71ae4 in qFatal(char const*, ...) () from /usr/lib64/qt4/libQtCore.so.4
#5  0x000000333b815fd3 in QWidgetPrivate::init(QWidget*, QFlags<Qt::WindowType>) () from /usr/lib64/qt4/libQtGui.so.4
#6  0x000000333b8165a7 in QWidget::QWidget(QWidget*, QFlags<Qt::WindowType>) () from /usr/lib64/qt4/libQtGui.so.4
#7  0x00007ffff78be032 in Breeze::PaletteHelper::PaletteHelper (this=0x913850, parent=<optimized out>, helper=...) at /var/src/kde/breeze/kstyle/breezepalettehelper.cpp:44
#8  0x00007ffff78cbf31 in Breeze::Style::Style (this=0x8997b0) at /var/src/kde/breeze/kstyle/breezestyle.cpp:165
#9  0x00007ffff78d50e4 in Breeze::StylePlugin::create (this=<optimized out>, key=...) at /var/src/kde/breeze/kstyle/breezestyleplugin.cpp:33
#10 0x000000333b9fe2c2 in QStyleFactory::create(QString const&) () from /usr/lib64/qt4/libQtGui.so.4
#11 0x000000333b7e4b24 in QApplication::style() () from /usr/lib64/qt4/libQtGui.so.4
#12 0x000000333b83494a in qt_set_x11_resources(char const*, char const*, char const*, char const*) () from /usr/lib64/qt4/libQtGui.so.4
#13 0x000000333b83a232 in qt_init(QApplicationPrivate*, int, _XDisplay*, unsigned long, unsigned long) () from /usr/lib64/qt4/libQtGui.so.4
#14 0x000000333b7e5490 in QApplicationPrivate::construct(_XDisplay*, unsigned long, unsigned long) () from /usr/lib64/qt4/libQtGui.so.4
#15 0x000000333b7e5b6a in QApplication::QApplication(int&, char**, int) () from /usr/lib64/qt4/libQtGui.so.4
#16 0x000000000040cf10 in main ()

The line to blame is line 5 below.

1. PaletteHelper::PaletteHelper( QObject* parent, Helper& helper ):
2.     QObject( parent ),
3.     _helper( helper )
4. {
5.     _widget = new QWidget();
6.     _widget->installEventFilter( this );
7. }
Comment 8 Hugo Pereira Da Costa 2015-01-23 14:40:19 UTC
mmm. Crap.
Which plasma application ? 
I cannot reproduce here (been running with breeze style from master on KDE4 everyday since the project started)
Comment 9 Hugo Pereira Da Costa 2015-01-23 14:41:24 UTC
also: could you install debug symbols for Qt4 so that we see exactly where the crash occur in QWidgetPrivate ?
Comment 10 Lamarque V. Souza 2015-01-23 14:57:01 UTC
any Qt4 application, not only Plasma. I have tested with qtconfig (from Qt 4.8.5), amarok, kwalletmanager, krita: 

[lamarque@evolucao ~]$ qtconfig
ASSERT: "allWidgets" in file kernel/qwidget.cpp, line 1286
Aborted

There is bug entry about a simular problem:

https://bugreports.qt.io/browse/QTBUG-38142
Comment 11 Lamarque V. Souza 2015-01-23 15:07:22 UTC
By the way, my main desktop is Plasma Desktop 5. I have not tested if breeze still crashes when not using Plasma 5 along with Qt4 applications.
Comment 12 Hugo Pereira Da Costa 2015-01-24 17:34:27 UTC
Hi Lamarque,
thank's for pointing to the Qt bug report. It is quite instructive. 
So I guess the fix, on my side is to create this new widget not in constructor, but the first time registerWidget is called. That would make sure that allWidgets is already initialized.
Will put a patch here for you to test and will push as soon as you confirm it does the job.
Comment 13 Hugo Pereira Da Costa 2015-01-24 17:40:30 UTC
Created attachment 90628 [details]
possible patch to crash reported by Lamarque
Comment 14 Lamarque V. Souza 2015-01-24 18:23:43 UTC
Hi, the patch works :-) Thanks for fixing the problem.
Comment 15 Hugo Pereira Da Costa 2015-01-24 18:37:16 UTC
Git commit 67bdd55531d3b46380dbc5cad3f76b0dda625340 by Hugo Pereira Da Costa.
Committed on 24/01/2015 at 17:38.
Pushed by hpereiradacosta into branch 'master'.

Initialize widget only at first successfull call to registerWidget

M  +8    -4    kstyle/breezepalettehelper.cpp
M  +1    -1    kstyle/breezepalettehelper.h

http://commits.kde.org/breeze/67bdd55531d3b46380dbc5cad3f76b0dda625340
Comment 16 Hugo Pereira Da Costa 2015-01-24 18:37:49 UTC
Git commit 5f77e60543e1d5820d6c4339d4d06b56dd2f74c9 by Hugo Pereira Da Costa.
Committed on 24/01/2015 at 17:38.
Pushed by hpereiradacosta into branch 'Plasma/5.2'.

Initialize widget only at first successfull call to registerWidget

M  +8    -4    kstyle/breezepalettehelper.cpp
M  +1    -1    kstyle/breezepalettehelper.h

http://commits.kde.org/breeze/5f77e60543e1d5820d6c4339d4d06b56dd2f74c9