> On Feb. 10, 2012, 12:01 a.m., Christoph Feck wrote: > > Thanks Albert for looking at it. Not sure if I understand everything > > correctly, but what happens, when I have a subclass of Q/KComboBox, that > > does not have its own user property? > > > > I am considering the following possible cases: > > > > 1) plain QComboBox > > 2) subclassed QComboBox without custom user property > > 3) subclassed QComboBox with custom user property > > 4) plain KComboBox > > 5) subclassed KComboBox without custom user property > > 6) subclassed KComboBox with custom user property (e.g. KColorCombo) > > > > For 1) 2) 4) 5) it should ignore the new 4.8 user property, and use our > > custom code. > > For 3) 6) it should respect the custom user property. > > > > If I am following code paths correctly, the patch fails for cases 2) and > > 5). It does not find the class name in the map, falls back to user property > > (what Qt provides now since 4.8), and thus not handle our custom code. > > Albert Astals Cid wrote: > "what happens, when I have a subclass of Q/KComboBox, that does not have > its own user property?" > I don't think that needs to be a supported use case at all, i.e. if you > don't tell KConfigDialogManager how to support your class, how are you > expecting it to work? By magic? > > Thus 2) and 5) are non-issues in my opinion > > Christoph Feck wrote: > To me it looks like the code part that starts with the > qobject_cast<QComboBox*> has been added to actually support combo boxes > without a user property. Qt did not handle them back then, now it does, but > differently than KDE handles them.
Yes, the code part that starts with the qobject_cast<QComboBox*> has been added to actually support combo boxes without a user property, and that is still what it does. - Albert ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/103909/#review10470 ----------------------------------------------------------- On Feb. 9, 2012, 11:28 p.m., Albert Astals Cid wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://git.reviewboard.kde.org/r/103909/ > ----------------------------------------------------------- > > (Updated Feb. 9, 2012, 11:28 p.m.) > > > Review request for kdelibs, Ben Cooksley, Eike Hein, Christoph Feck, and > Jeremy Paul Whiting. > > > Description > ------- > > https://git.reviewboard.kde.org/r/101486/ broke subclasses of QComboBox that > have a USER property like KColorCombo, this patch reverts this change and > introduces a different code path to ignore the USER property of QComboBox and > KComboBox and make it use our custom code. > > > This addresses bug 293702. > http://bugs.kde.org/show_bug.cgi?id=293702 > > > Diffs > ----- > > kdeui/tests/CMakeLists.txt 63788f6 > kdeui/tests/kconfigdialog_unittest.cpp PRE-CREATION > kdeui/dialogs/kconfigdialogmanager.cpp 0890c0b > > Diff: http://git.reviewboard.kde.org/r/103909/diff/ > > > Testing > ------- > > Ran the attached test, everything worked. > > Without moving the > userproperty = getUserProperty(w); > the KColorCombo fails > > Without adding the > s_propertyMap->insert( "KComboBox", "" ); > the editable KComboBox fails > > > Thanks, > > Albert Astals Cid > >
