[webkit-reviews] review denied: [Bug 26988] Haiku-specific files for WebCore : [Attachment 32869] Patch to add a fourth bunch of Haiku-specific files for WebCore.

[bugzilla-daemon at webkit.org]

David Levin <levin at chromium.org> has denied Maxime Simon
<simon.maxime at gmail.com>'s request for review:
Bug 26988: Haiku-specific files for WebCore
https://bugs.webkit.org/show_bug.cgi?id=26988

Attachment 32869: Patch to add a fourth bunch of Haiku-specific files for
WebCore.
https://bugs.webkit.org/attachment.cgi?id=32869&action=review

------- Additional Comments from David Levin <levin at chromium.org>
Just a few things to fix up here.


Please run check-webkit-style on this files in this patch to point out the TABs
and unsorted header files, etc.  Since the tool will point them out, I'll not
enumerate these issues.


> diff --git a/WebCore/platform/haiku/PasteboardHaiku.cpp
b/WebCore/platform/haiku/PasteboardHaiku.cpp

> +void Pasteboard::writeSelection(Range* selectedRange, bool
canSmartCopyOrDelete, Frame* frame)
> +{
> +    BClipboard clipboard("WebKit");
> +    BMessage* data;
> +
> +    if (clipboard.Lock()) {

fwiw, WebKit tends to favor a return early, so
if (!clipboard.Lock())
    return;
would follow this style better.


> +	   clipboard.Clear();

It is nice to define types in the smallest possible scope and to initialize
them when declared.

So I'd suggestion move BMessage* here.

    BMessage* data = clipboard.Data();
    if (data) {

Same thing for other similar code in here.

>
> diff --git a/WebCore/platform/haiku/PlatformKeyboardEventHaiku.cpp
b/WebCore/platform/haiku/PlatformKeyboardEventHaiku.cpp
> +	    //case B_SPACE:
> +	    //	return "";
> +	    case B_TAB:
> +		return "U+0009";
> +	    //case B_ESCAPE:
> +	    //	return "";

Typical WebKit style is to not check in commented out code.

> +	   return( 0 );

Just "return 0;"

> +    default:
> +	   return( 0 );

Just "return 0;"

> +PlatformKeyboardEvent::PlatformKeyboardEvent(BMessage *message)
"*" placement.	"BMessage*"


> diff --git a/WebCore/platform/haiku/PlatformWheelEventHaiku.cpp
b/WebCore/platform/haiku/PlatformWheelEventHaiku.cpp
> +PlatformWheelEvent::PlatformWheelEvent(BMessage *message)

"*" placement.	"BMessage*"