> On Oct. 18, 2015, 1:52 a.m., Michael Pyne wrote: > > I agree that the current code is broken, and that your fix makes sense > > given the surrounding comments clarifying ownership. But why not simply > > construct the print settings object with the dialog as a parent? I see no > > reason the dialog can't be constructed first and then the settings, and > > with the transfer in ownership there's no reason to have a dedicated > > deleter just for the settings page. > > Robby Stephenson wrote: > I agree. Would you like me to adjust the patch accordingly? I just went > for the minimalist fix here. I wasn't sure if there were any other factors I > didn't see...
Yeah, I'd give it a shot and as long as you're able to print, then it's at least still a large improvement. - Michael ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/125681/#review86991 ----------------------------------------------------------- On Oct. 18, 2015, 1:13 a.m., Robby Stephenson wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/125681/ > ----------------------------------------------------------- > > (Updated Oct. 18, 2015, 1:13 a.m.) > > > Review request for KDE Frameworks and Martin Tobias Holmedahl Sandsmark. > > > Repository: khtml > > > Description > ------- > > Avoid setting printSettings as its own parent. It should properly be > owned by the print dialog, which seems to have been the intent in KDE4. > > > Diffs > ----- > > src/khtmlview.cpp d9bbc38b1677a3faf3be46ccc3a211c8d7289e45 > > Diff: https://git.reviewboard.kde.org/r/125681/diff/ > > > Testing > ------- > > Tellico's printing (which uses KHTMLPart) no longer hangs, but works properly. > > > Thanks, > > Robby Stephenson > >
_______________________________________________ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
