----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviewboard.kde.org/r/4124/#review5859 -----------------------------------------------------------
having the header there is intentional and desired. the point is to show the name of the page there if a replacement string isn't explicitly defined. i'm not sure what problem is trying to be solved here, exactly, but this changes the behaviour of this dialog everywhere and isn't acceptable as such. perhaps we can discuss on plasma-devel@ what the actual goal here is with this patch and try to come up with a workable solution. - Aaron On 2010-05-24 16:55:36, Ignat Semenov wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://reviewboard.kde.org/r/4124/ > ----------------------------------------------------------- > > (Updated 2010-05-24 16:55:36) > > > Review request for kdelibs and Plasma. > > > Summary > ------- > > This is essentially a workaround for the broken logic in > KPageWidgetItem::setHeader(). When no header text was supplied in > KConfigDialog::addPage(), a KPageWidgetItem was created with QString() as the > header text, which caused the header to be set, as setHeader checks for > header.isNull() and thus requires a QString(""). This patch makes sure that > when header==QString() in addPage, setHeader( QString("") ) is called. > > > Diffs > ----- > > /trunk/KDE/kdelibs/kdeui/dialogs/kconfigdialog.cpp 1130107 > > Diff: http://reviewboard.kde.org/r/4124/diff > > > Testing > ------- > > After applying the patch, no headers appear in the Plasma configuration > dialogs, which use KConfigDialog::addPage(QWidget *, QString &, QString &). > It doesn't break BC as the cnahge is done in a private class. > > > Thanks, > > Ignat > > _______________________________________________ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
