[webkit-reviews] review denied: [Bug 35844] [Qt] Add support for LayoutTestController commands. : [Attachment 50960] Updated Patch - with QDRTSupport

[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 50960: Updated Patch - with QDRTSupport
https://bugs.webkit.org/attachment.cgi?id=50960&action=review

------- Additional Comments from Simon Hausmann <hausmann at webkit.org>

> diff --git a/WebKit/qt/ChangeLog b/WebKit/qt/ChangeLog
> index 546ac34..e73082a 100644
> --- a/WebKit/qt/ChangeLog
> +++ b/WebKit/qt/ChangeLog
> @@ -1,3 +1,49 @@
> +2010-03-07  Robert Hogan  <robert at webkit.org>
> +
> +	   Reviewed by NOBODY (OOPS!).
> +
> +	   [Qt] Add support for LayoutTestController commands.
> +
> +	   Add support for LayoutTestController commands:
> +	     setSmartInsertDeleteEnabled
> +	     setSelectTrailingWhitespaceEnabled
> +	     execCommand
> +	     isCommandEnabled
> +
> +	   Unskip tests:
> +	   editing/deleting/smart-editing-disabled.html
> +	   editing/execCommand/19089.html
> +	   editing/execCommand/delete-image-in-anchor.html
> +	   editing/execCommand/enabling-and-selection-2.html
> +	   editing/selection/doubleclick-whitespace-crash.html
> +	   editing/selection/doubleclick-whitespace-img-crash.html
> +	   editing/selection/doubleclick-whitespace.html
> +	   editing/selection/select-line.html
> +	   editing/deleting/5300379.html
> +	   editing/deleting/smart-delete-003.html
> +	   editing/deleting/smart-delete-004.html
> +	   editing/selection/5195166-1.html
> +	   editing/selection/5195166-2.html
> +
> +	   https://bugs.webkit.org/show_bug.cgi?id=35844
> +
> +	   * Api/qwebpage.cpp:
> +	   (qt_drt_executeCoreCommandByName):
> +	   (qt_drt_isCommandEnabled):
> +	   (QWebPagePrivate::QWebPagePrivate):
> +	   (QWebPage::setSmartInsertDeleteEnabled):
> +	   (QWebPage::smartInsertDeleteEnabled):
> +	   (QWebPage::toggleSmartInsertDeleteEnabled):
> +	   (QWebPage::setSelectTrailingWhitespaceEnabled):
> +	   (QWebPage::selectTrailingWhitespaceEnabled):
> +	   * Api/qwebpage.h:
> +	   * Api/qwebpage_p.h:
> +	   * WebCoreSupport/EditorClientQt.cpp:
> +	   (WebCore::EditorClientQt::smartInsertDeleteEnabled):
> +	   (WebCore::EditorClientQt::toggleSmartInsertDelete):
> +	   (WebCore::EditorClientQt::isSelectTrailingWhitespaceEnabled):
> +	   * WebCoreSupport/EditorClientQt.h:
> +

It looks like the ChangeLog is missing the qt_drt_* refactoring.

>  2010-03-17  Csaba Osztrogonác  <ossy at webkit.org>
>  
>	   Reviewed by Kenneth Rohde Christiansen.
> diff --git a/WebKit/qt/WebCoreSupport/DumpRenderTreeSupport.cpp
b/WebKit/qt/WebCoreSupport/DumpRenderTreeSupport.cpp
> new file mode 100644
> index 0000000..ccf13a3
> --- /dev/null
> +++ b/WebKit/qt/WebCoreSupport/DumpRenderTreeSupport.cpp
> @@ -0,0 +1,357 @@
> +/*
> + * Copyright (C) 2007 Apple Inc.  All rights reserved.
> + *
> + * Redistribution and use in source and binary forms, with or without
> + * modification, are permitted provided that the following conditions
> + * are met:
> + * 1. Redistributions of source code must retain the above copyright
> + *	 notice, this list of conditions and the following disclaimer.
> + * 2. Redistributions in binary form must reproduce the above copyright
> + *	 notice, this list of conditions and the following disclaimer in the
> + *	 documentation and/or other materials provided with the distribution.
> + *
> + * THIS SOFTWARE IS PROVIDED BY APPLE COMPUTER, INC. ``AS IS'' AND ANY
> + * EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
> + * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR
> + * PURPOSE ARE DISCLAIMED.  IN NO EVENT SHALL APPLE COMPUTER, INC. OR
> + * CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL,
> + * EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO,
> + * PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR
> + * PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY
> + * OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
> + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
> + * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. 
> + */

This doesn't look like the right copyright / license. The code comes mostly
from qweb* classes and you should add your name, too, I think.


Otherwise this looks great!