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

_______________________________________________
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Reply via email to