[Webkit-unassigned] [Bug 73143] Upstream BlackBerry porting of page

[bugzilla-daemon at webkit.org]

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


Daniel Bates <dbates at webkit.org> changed:

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




--- Comment #7 from Daniel Bates <dbates at webkit.org>  2011-11-25 15:40:42 PST ---
(From update of attachment 116640)
View in context: https://bugs.webkit.org/attachment.cgi?id=116640&action=review

This change looks good. I have some minor issues.

> Source/WebCore/page/blackberry/AccessibilityObjectBlackBerry.cpp:26
> +namespace WebCore {
> +
> +} // namespace WebCore

Disregarding the #includes and copyright, this file is empty. Do we need it?

> Source/WebCore/page/blackberry/DragControllerBlackBerry.cpp:21
> +#if ENABLE(DRAG_SUPPORT)
> +#include "DragController.h"

Swap these lines.

> Source/WebCore/page/blackberry/DragControllerBlackBerry.cpp:25
> +#include "IntSize.h"

This include is unnecessary since DragController.h includes IntPoint.h that includes this file.

> Source/WebCore/page/blackberry/DragControllerBlackBerry.cpp:32
> +// FIXME: remove this when no stubs use it anymore
> +static const WebCore::IntSize s_nullSize = WebCore::IntSize();

We should move this into the body of DragController::maxDragImageSize(). Also, use DEFINE_STATIC_LOCAL to define this static variable. See <http://trac.webkit.org/browser/trunk/Source/JavaScriptCore/wtf/StdLibExtras.h?rev=97675#L32> for more details.

> Source/WebCore/page/blackberry/DragControllerBlackBerry.cpp:34
> +const double EventHandler::TextDragDelay = 0.0;

0.0 => 0

> Source/WebCore/page/blackberry/EventHandlerBlackBerry.cpp:23
> +#include "Clipboard.h"

This include is unnecessary as ClipboardBlackBerry.h (included below) includes this file.

> Source/WebCore/page/blackberry/EventHandlerBlackBerry.cpp:32
> +#include "PlatformMouseEvent.h"

This include is unnecessary as MouseEventWithHitTestResults.h (included above) includes this file.

> Source/WebCore/page/blackberry/EventHandlerBlackBerry.cpp:68
> +bool EventHandler::passWidgetMouseDownEventToWidget(MouseEventWithHitTestResults const&)

This signature doesn't match the signature for passWidgetMouseDownEventToWidget() in EventHandler.h: <http://trac.webkit.org/browser/trunk/Source/WebCore/page/EventHandler.h?rev=100483#L298>.

> Source/WebCore/page/blackberry/EventHandlerBlackBerry.cpp:79
> +unsigned int EventHandler::accessKeyModifiers()

unsigned int => unsigned

> Source/WebCore/page/blackberry/FrameBlackBerry.cpp:26
> +void* Frame::dragImageForSelection()

void* => DragImageRef. Then typedef DragImageRef in WebCore/platform/DragImage.h: <http://trac.webkit.org/browser/trunk/Source/WebCore/platform/DragImage.h?rev=95922#L65>.

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