----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/102246/#review5532 -----------------------------------------------------------
API looks ok. Implementation could prevent duplication by making a short file-static function like static void applyOptions(dialog, options) { [the new code] dialog->setModal( true ); } This will also make it easier for the next time :-) - David On Aug. 9, 2011, 8:39 a.m., David Jarvie wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://git.reviewboard.kde.org/r/102246/ > ----------------------------------------------------------- > > (Updated Aug. 9, 2011, 8:39 a.m.) > > > Review request for kdelibs. > > > Summary > ------- > > According to the apidocs, KMessageBox instances with a parent widget > specified are supposed to be window modal, not application modal. This patch > fixes this. > > > Diffs > ----- > > kdeui/dialogs/kmessagebox.cpp 939be89 > kdeui/dialogs/kmessagebox.h 286880e > > Diff: http://git.reviewboard.kde.org/r/102246/diff > > > Testing > ------- > > Tested warningYesNo(), questionYesNoCancel() with a parent widget - widgets > in a different window tree were still able to be used. > > kmessageboxtest runs ok. > > > Thanks, > > David > >