Review Request 125681: Correctly set the printSetting's parent

[Michael Pyne]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/125681/#review86991
-----------------------------------------------------------

Ship it!


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.

- Michael Pyne


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

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20151018/16b2cca9/attachment.html>