----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/108637/#review26294 -----------------------------------------------------------
kdeui/colors/kcolorbutton.cpp <http://git.reviewboard.kde.org/r/108637/#comment20013> Perhaps this signal/slot is not necessary because okClicked() call accept() which emit accepted() see: void QDialog::done(int r) { .... emit finished(r); if (r == Accepted) emit accepted(); ... } so for me we can remove this line. - Laurent Montel On Jan. 28, 2013, 12:27 a.m., Albert Astals Cid wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://git.reviewboard.kde.org/r/108637/ > ----------------------------------------------------------- > > (Updated Jan. 28, 2013, 12:27 a.m.) > > > Review request for kdelibs and Laurent Montel. > > > Description > ------- > > Do not connect colorSelected to _k_colorChosen otherwise two bad things > happen: > a) Selecting a color and canceling still changes the color of the color > button (this is KColorButtonTest::testChangeAndCancel that fails without my > change) > b) If you are in Recent Colors there's a crash (bug 313984 because a lot of > bad things happen (this is not in my tests, will add later if you guys agree > this is the correct fix)) > > Laurent added > connect(dialog, SIGNAL(colorSelected(QColor)), q, > SLOT(_k_colorChosen())); > to make sure that double click worked, thus i've added > KColorButtonTest::testDoubleClickChange() to prove it still works after my > change > > Comments? > > Developed against master, plan to apply against KDE/4.10 too > > > Diffs > ----- > > kdeui/colors/kcolorbutton.cpp 786cb9d > kdeui/tests/CMakeLists.txt 85f12ed > kdeui/tests/kcolorbuttontest.h PRE-CREATION > kdeui/tests/kcolorbuttontest.cpp PRE-CREATION > > Diff: http://git.reviewboard.kde.org/r/108637/diff/ > > > Testing > ------- > > Okular no longer crashes, the tests > > > Thanks, > > Albert Astals Cid > >
