Review Request: Properly hide KConfigDialog page headers when there is no header text set

[Aaron Seigo]

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