Review Request 125681: Correctly set the printSetting's parent

[Michael Pyne]

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

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