[webkit-reviews] review granted: [Bug 23328] Upstream remaining files from platform/chromium/ : [Attachment 26721] v1 patch
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Wed Jan 14 11:34:06 PST 2009
Eric Seidel <eric at webkit.org> has granted Darin Fisher (Google)
<darin at chromium.org>'s request for review:
Bug 23328: Upstream remaining files from platform/chromium/
https://bugs.webkit.org/show_bug.cgi?id=23328
Attachment 26721: v1 patch
https://bugs.webkit.org/attachment.cgi?id=26721&action=review
------- Additional Comments from Eric Seidel <eric at webkit.org>
notImplemented()?
IntSize dragImageSize(DragImageRef image)
39 {
40 return IntSize();
41 }
42
Other code seems to prefer just "unsigned" over "unsigned int":
36 int windowsKeyCodeForKeyEvent(unsigned int keycode);
It's a little funny just to wrap this one line:
2 ChromiumBridge::clipboardWriteSelection(html, url, plainText,
83 canSmartCopyOrDelete);
"event" is not needed here:
78 virtual bool handleMouseDownEvent(const PlatformMouseEvent& event);
79 virtual bool handleMouseMoveEvent(const PlatformMouseEvent& event);
80 virtual bool handleMouseReleaseEvent(const PlatformMouseEvent& event);
81 virtual bool handleWheelEvent(const PlatformWheelEvent& event);
82 virtual bool handleKeyEvent(const PlatformKeyboardEvent& event);
Generally I've seen WebCore avoid this kind of construction:
894 m_originalIndex = m_selectedIndex = index;
But I'm not sure we have any specific style rules about it.
No longer required after your previous change:
// Required to use notImplemented() outside of the WebCore namespace.
49 using namespace WebCore;
I have no substantive comments. This looks fine as-is.
More information about the webkit-reviews
mailing list