[Webkit-unassigned] [Bug 86296] Implement LayoutTest's eventSender.beginDragWithFiles interface in windows platform

[bugzilla-daemon at webkit.org]

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





--- Comment #6 from huangxueqing <huangxueqing at baidu.com>  2012-05-23 06:15:55 PST ---
(In reply to comment #4)
> (From update of attachment 141580 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=141580&action=review
> 
> Please rebase and run check-webkit-style.  I started to review, but there are too many style errors.
> 
> > LayoutTests/ChangeLog:9
> > +        * editing/pasteboard/drag-and-drop-file-to-input.html: Added.
> > +        * platform/win/editing/pasteboard/drag-and-drop-file-to-input-expected.txt: Added.
> 
> You don't need to add a new test case for this, although I would expect the win/Skipped file to be updated as this will probably cause some tests to pass.
Well, I weill remove it.

> 
> > LayoutTests/editing/pasteboard/drag-and-drop-file-to-input.html:29
> > +    for (var i=0; i<fileInput.files.length; ++i)
> > +    {
> 
> Spaces around =, spaces around <, { goes on previous line.
> 
> > Source/WebCore/platform/win/DragDataWin.cpp:116
> > +    if (m_platformDragData) {
> > +        STGMEDIUM medium;
> 
> Nit: WebKit tends to use early returns rather than indenting the whole method.
Done

> 
> > Source/WebCore/platform/win/DragDataWin.cpp:120
> > +        HDROP hdrop = (HDROP)GlobalLock(medium.hGlobal);
> 
> Please use static_cast<HDROP>() instead of (HDROP).
Done

> 
> > Tools/DumpRenderTree/win/DRTDataObject.cpp:62
> > +    Vector<FORMATETC>  m_formats;
> 
> Nit: Why 2 spaces?
Done

> 
> > Tools/DumpRenderTree/win/DRTDataObject.cpp:70
> > +: m_ref(1)
> > +, m_current(0)
> 
> Nit: Indent 4 spaces
Done

> 
> > Tools/DumpRenderTree/win/DRTDataObject.cpp:80
> > +    for(size_t i = 0; i < formats.size(); ++i)
> 
> Nit: Space after 'for'
Done

> 
> > Tools/DumpRenderTree/win/DRTDataObject.cpp:84
> > +STDMETHODIMP  WCEnumFormatEtc::QueryInterface(REFIID riid, void** ppvObject)
> 
> Nit: 2 spaces?
Done

> 
> > Tools/DumpRenderTree/win/DRTDataObject.cpp:103
> > +    long c = InterlockedDecrement(&m_ref);
> 
> Please use a longer variable name than 'c'.
Done

> 
> > Tools/DumpRenderTree/win/DRTDataObject.cpp:109
> > +STDMETHODIMP WCEnumFormatEtc::Next(ULONG celt, LPFORMATETC lpFormatEtc, ULONG* pceltFetched)
> 
> What is celt?
celf is the number of FORMATETC items in the IEnumFORMATETC interface, please see http://msdn.microsoft.com/en-us/library/windows/desktop/dd542673(v=vs.85).aspx

> 
> > Tools/DumpRenderTree/win/DRTDataObject.cpp:111
> > +    if(pceltFetched != 0)
> 
> Space after 'if'
Done

> 
> > Tools/DumpRenderTree/win/DRTDataObject.cpp:116
> > +    if(celt <= 0 || lpFormatEtc == 0 || m_current >= m_formats.size())
> 
> Space after 'if'
Done

> 
> > Tools/DumpRenderTree/win/DRTDataObject.cpp:119
> > +    if(pceltFetched == 0 && celt != 1) // pceltFetched can be 0 only for 1 item request
> 
> Space after 'if'
> 
> > Tools/DumpRenderTree/win/DRTDataObject.cpp:126
> > +    if (pceltFetched != 0)
> 
> if (pceltFetched)
> 
> > Tools/DumpRenderTree/win/DRTDataObject.cpp:148
> > +    if(!ppCloneEnumFormatEtc)
> 
> Space after 'if'
> 
> > Tools/DumpRenderTree/win/DRTDataObject.cpp:152
> > +    if(!newEnum)
> 
> Space after 'if'
Done

> 
> > Tools/DumpRenderTree/win/DRTDataObject.cpp:174
> > +: m_ref(1)
> 
> indent 4
Done

> 
> > Tools/DumpRenderTree/win/DRTDataObject.cpp:180
> > +    for(size_t i = 0; i < m_medium.size(); ++i) {
> 
> Space after 'for'
Done

> 
> > Tools/DumpRenderTree/win/DRTDataObject.cpp:338
> > +    *ppenumFormatEtc=0;
> 
> Spaces around =
Done

> 
> > Tools/DumpRenderTree/win/DRTDataObject.cpp:342
> > +        *ppenumFormatEtc= new WCEnumFormatEtc(m_formats);
> 
> space before =
Done

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