[webkit-reviews] review granted: [Bug 60819] REGRESSION (WK2): Can't drag and drop a link or image from Safari to Desktop : [Attachment 93534] Patch
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Fri May 13 17:16:07 PDT 2011
Darin Adler <darin at apple.com> has granted Enrica Casucci <enrica at apple.com>'s
request for review:
Bug 60819: REGRESSION (WK2): Can't drag and drop a link or image from Safari to
Desktop
https://bugs.webkit.org/show_bug.cgi?id=60819
Attachment 93534: Patch
https://bugs.webkit.org/attachment.cgi?id=93534&action=review
------- Additional Comments from Darin Adler <darin at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=93534&action=review
> Source/WebCore/platform/win/ClipboardUtilitiesWin.cpp:374
> + static FORMATETC fileDescriptorFormat = {cf, 0, DVASPECT_CONTENT, -1,
TYMED_HGLOBAL};
WebKit coding style puts spaces at the braces.
> Source/WebCore/platform/win/ClipboardUtilitiesWin.cpp:381
> + static FORMATETC fileContentFormat = {cf, 0, DVASPECT_CONTENT, 0,
TYMED_HGLOBAL};
WebKit coding style puts spaces at the braces.
> Source/WebCore/platform/win/ClipboardUtilitiesWin.cpp:392
> + FILEGROUPDESCRIPTOR* fgd =
(FILEGROUPDESCRIPTOR*)::GlobalLock(store.hGlobal);
Could we use a C++ cast here?
> Source/WebCore/platform/win/ClipboardUtilitiesWin.cpp:393
> + size = fgd->fgd[0].nFileSizeLow;
What if the size is larger than 31 bits?
> Source/WebCore/platform/win/ClipboardUtilitiesWin.cpp:414
> + STGMEDIUM medium = {0};
The braces thing again.
> Source/WebCore/platform/win/ClipboardUtilitiesWin.cpp:421
> + FILEGROUPDESCRIPTOR* fgd =
(FILEGROUPDESCRIPTOR*)::GlobalLock(medium.hGlobal);
Could we use a C++ cast here?
> Source/WebCore/platform/win/ClipboardUtilitiesWin.cpp:427
> + wcscpy(fgd->fgd[0].cFileName, pathname.charactersWithNullTermination());
What guarantees that cFileName is big enough to hold pathname without
overrunning the buffer?
> Source/WebCore/platform/win/ClipboardUtilitiesWin.cpp:435
> + STGMEDIUM medium = {0};
Braces again.
>> Source/WebCore/platform/win/ClipboardUtilitiesWin.h:81
>> +void getFileDescriptorData(IDataObject* dataObject, int& size, String&
pathname);
>
> The parameter name "dataObject" adds no information, so it should be removed.
[readability/parameter_name] [5]
I agree with the style queue on this.
> Source/WebCore/platform/win/ClipboardWin.cpp:221
> + OutputDebugString(L"FILE WRITTEN\n");
Did you mean to land this?
> Source/WebKit2/WebProcess/WebCoreSupport/win/WebDragClientWin.cpp:96
> + m_page->send(Messages::WebPageProxy::StartDragDrop(imageOrigin,
dragPoint, okEffect, dragData.dragDataMap(), (uint64_t)fileSize, pathname,
fileContentHandle, IntSize(bitmapInfo.bmiHeader.biWidth,
bitmapInfo.bmiHeader.biHeight), handle, isLink), m_page->pageID());
Do we need that typecast to uint64_t?
More information about the webkit-reviews
mailing list