[Webkit-unassigned] [Bug 26988] Haiku-specific files for WebCore

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=26988


David Levin <levin at chromium.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #32868|review?                     |review-
               Flag|                            |




--- Comment #31 from David Levin <levin at chromium.org>  2009-07-19 23:37:10 PDT ---
(From update of attachment 32868)

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

> +#include "HashTable.h"

Should be "#include <wtf/HashTable.h>"

> +
> +
> +namespace WebCore {
> +
> +ClipboardHaiku::ClipboardHaiku(ClipboardAccessPolicy policy, bool forDragging) 
> +    : Clipboard(policy, forDragging)
> +{
> +}
> +
> +void ClipboardHaiku::clearData(const String& type)
> +{
> +    BMessage *data = (BMessage *) NULL;
1. Star in wrong place.
BMessage* data

2. Use c++ style casts. (BMessage *)  (but I don't think the cast is necessary.

3. Use 0 instead of NULL.

This line is repeated a lot in this patch, but I'm only flagging this first
instace.


> +
> +    if (be_clipboard->Lock()) {
> +        if ((data = be_clipboard->Data())) {
Since there doesn't seem to be any benefit of putting this in the if other than
saving a line.  Why not write it as two lines (which in my opinion makes it
more readable)?

  data = be_clipboard->Data();
  if (data) {

It isn't necessary to be this but it would be nice imo.



> +// extensions beyond IE's API
Format comments like sentences (begin with a capital and end with a period).

> +IntPoint ClipboardHaiku::dragLocation() const 
> +{ 
> +    notImplemented();
> +    return IntPoint(0,0);
Add a space after the comma.


> +DragImageRef ClipboardHaiku::createDragImage(IntPoint& dragLoc) const
Avoid abbreviations: dragloc -> dragLocation

> diff --git a/WebCore/platform/haiku/ClipboardHaiku.h b/WebCore/platform/haiku/ClipboardHaiku.h
> +    // State available during IE's events for drag and drop and copy/paste
> +    class ClipboardHaiku : public Clipboard {
> +    public:
> +        ClipboardHaiku(ClipboardAccessPolicy policy, bool forDragging);

"bool forDragging"
Please use an enum instead of a bool. (See
http://trac.webkit.org/browser/trunk/WebCore/workers/WorkerScriptLoader.h for
an example of where this was done.)



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

> +namespace WebCore {
> +
> +Cursor::Cursor(PlatformCursor p)
"p" -- Use words instead f abbreviations for variable names.


> +static Cursor global_cursor((PlatformCursor)B_CURSOR_SYSTEM_DEFAULT);

Improper casing global_cursor -> globalCursor
Use C++ style casting (static_cast/reinterpret_cast, etc.) instead of C style
casting.


> +static Cursor ibeam_cursor((PlatformCursor)B_CURSOR_I_BEAM);

Same comments as above.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.