Review Request 101486 (Q/KComboBox related KConfigDialogManager change) breaks apps

[Christoph Feck]

On Wednesday 30 November 2011 21:50:41 Andreas Pakulat wrote:
> Hi,
> 
> just came around to notice this now, but the mentioned code-change
> from the review request: https://git.reviewboard.kde.org/r/101486/
> actually does break apps. In particular it breaks kdevelop. We
> have a KComboBox subclass which actually wants to store a custom
> string-property into the kconfig file and not an index. At the same
> time the combobox is not editable, but the strings are
> user-supplied (in a separate widget somewhere else). So storing
> the string is fine and actually necessary since the order is
> undefined over restarts.
> 
> The commit 8edc1932ecc62370d9a31836dfa9b2bd0175a293 and
> d44186bce4670d2985fb6aba8dba59bbd2c4c77a changed the
> kconfigdialogmanager to first check for q/kcombobox style and then
> enforces using the index or the text depending on the
> editable-flag. The reason for that is that QCombobox in Qt 4.8
> changed its behaviour and exposes its currentIndex roperty as a
> user-proeprty. This leads to storing the index of user-editable
> comboboxes instead of the text, which is wrong and even dangerous.
> IMHO it should be reverted in Qt, but I doubt its going to happen
> and hence not worth spending time on.

That problem was previously reported here, thanks for reminding us :)

> 
> Since nonetheless there's the case of breaking existing apps with
> this change, I'd like to check other peoples opinions on adding
> some more logic to the kconfigdialogmanager's code so that if the
> user-property comes from a pure Q/KCombobox (i.e. not a subclass)
> and is currentIndex its ignored, otherwise its taken account.

I did not yet find a way to see if the user property actually comes 
from QComboBox, or a subclass. I guess the Qt property system has no 
way to find that out, but if you find a way, please share a patch.

> 
> I'm not sending a review-request yet since I don't have a patch and
> first have to setup a kdelibs dev env anyway. So if there are
> objections to adding such logic I'd like to spare the time for the
> setup.
> 
> For KDevelop we have a workaround now by setting the kcfg_property
> special property which again overrules the hardcoded handling for
> Q/KComboBox.
> 
> Andreas

Christoph Feck (kdepepo)
KDE Quality Team