[webkit-reviews] review denied: [Bug 29248] [Qt] [API] Make it possible to have 'invisible' loads : [Attachment 39630] patch 0.2.1 - same functionality as 0.2, but right changelog

[bugzilla-daemon at webkit.org]

Simon Hausmann <hausmann at webkit.org> has denied  review:
Bug 29248: [Qt] [API] Make it possible to have 'invisible' loads
https://bugs.webkit.org/show_bug.cgi?id=29248

Attachment 39630: patch 0.2.1 - same functionality as 0.2, but right changelog
https://bugs.webkit.org/attachment.cgi?id=39630&action=review

------- Additional Comments from Simon Hausmann <hausmann at webkit.org>
I'm going to say r-, but I agree with the concept and the need to have this
functionality.

First of all this patch is lacking an autotest. For changes in the loader, etc.
we really need an automated test for this.

I also admit that I'm not happy about the API. setHtml becomes inconsistent to
setContent and with four arguments
it becomes quite ugly.

If we decide not to change setHtml's behaviour, then I wonder if it would be
easier to have a dedicated function for
the job needed, instead of trying to merge this into the setHtml function.

Does the error message that you'd like to display as substitute need to be
generated dynamically? Could it be a property
on the QWebPage?

Would it make sense to have an extension in QWebPage that allows the
application to provide substitute content
in case loading of a url fails?