Bug 341940

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 ...

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.

(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.

see 4bfd9c3f9e1ec4333027e1c780dc339e5700ae4d

In fact, the issue might well be that Style::polish( QApplication*) is simply not called (or at least not for some apps)

(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

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

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. }

mmm. Crap.
Which plasma application ? 
I cannot reproduce here (been running with breeze style from master on KDE4 everyday since the project started)

also: could you install debug symbols for Qt4 so that we see exactly where the crash occur in QWidgetPrivate ?

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

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.

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.

Created attachment 90628 [details]
possible patch to crash reported by Lamarque

Hi, the patch works :-) Thanks for fixing the problem.

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

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