[webkit-reviews] review denied: [Bug 35844] [Qt] Add support for LayoutTestController commands. : [Attachment 50171] Patch

[bugzilla-daemon at webkit.org]

Simon Hausmann <hausmann at webkit.org> has denied Robert Hogan
<robert at webkit.org>'s request for review:
Bug 35844: [Qt] Add support for LayoutTestController commands.
https://bugs.webkit.org/show_bug.cgi?id=35844

Attachment 50171: Patch
https://bugs.webkit.org/attachment.cgi?id=50171&action=review

------- Additional Comments from Simon Hausmann <hausmann at webkit.org>
Looks good in general, yay for passing more tests.

The part I'm unhappy about are the functions added to QWebPage:

* We usually don't have toggle() functions for plain boolean properties.
* The added properties are lacking documentation!
* I'd like to see the names discussed/proposed on the mailing list first, for a
wider review.
* The selectTrailingWhitespaceEnabled property is lacking a Q_PROPERTY
declaration

An alternative to the public API would and a cleanup of all the _drt_ functions
would be to collect all those ugly qt_drt_foo functions, put them into
WebCoreSupport/DumpRenderTreeSupport.cpp/h, _export_ the entire class and
access it from the DRT by including the private header file.