[Webkit-unassigned] [Bug 116258] [WIN] Move Windows port off legacy clipboard

[bugzilla-daemon at webkit.org]

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





--- Comment #9 from huangxueqing <huangxueqing at baidu.com>  2013-05-24 01:09:58 PST ---
(In reply to comment #7)
> (From update of attachment 202492 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=202492&action=review
> 
> Great patch! Definitely a step in the right direction. A few things that need fixing, but I think it’s getting close!
> 
> > Source/WebCore/page/win/EventHandlerWin.cpp:46
> > +#if !USE(LEGACY_STYLE_ABSTRACT_CLIPBOARD_CLASS)
> 
> Should not use this #if in this file, since it’s only used for PLATFORM(WIN). Should just keep the !USE(LEGACY_STYLE_ABSTRACT_CLIPBOARD_CLASS) and delete the other side.
> 
Done

> > Source/WebCore/platform/Pasteboard.h:72
> > +#if PLATFORM(WIN)
> > +class Element;
> > +#endif
> 
> Should not put an #if around a forward declaration. Just leave the #if off.
> 
Done

> > Source/WebCore/platform/Pasteboard.h:171
> > +    void declareAndWriteDragImage(Element*, const KURL&, const String&, Frame*);
> 
> This function does not belong here. Pasteboard should expose what’s needed to implement this, but this function should stay elsewhere. I think DragControllerWin is the right file for this or we could leave it in ClipboardWin for now.
> 
Done

> > Source/WebCore/platform/win/EditorWin.cpp:30
> > +#if !USE(LEGACY_STYLE_ABSTRACT_CLIPBOARD_CLASS)
> 
> Should not use this #if in this file, since it’s only used for PLATFORM(WIN). Should just keep the !USE(LEGACY_STYLE_ABSTRACT_CLIPBOARD_CLASS) and delete the other side.
> 
Done

> > Source/WebCore/platform/win/EditorWin.cpp:62
> > +    Pasteboard* pasteboard = const_cast<Pasteboard*>(&clipboard->pasteboard());
> > +    pasteboard->setExternalDataObject(clipboardData.get());
> 
> Ultimately this const_cast should not be needed. There’s some kind of design mistake that makes it needed for now. But no need for the pointer, this should be:
> 
>     const_cast<Pasteboard&>(clipboard->pasteboard()).setExternalDataObject(clipboardData.get());
> 
Done

> > Source/WebCore/platform/win/PasteboardWin.cpp:111
> > +// static
> 
> We don't put in these kinds of comments in WebKit coding style.
> 
Done

> > Source/WebCore/platform/win/PasteboardWin.cpp:130
> > +void Pasteboard::createPasteboardPrivate()
> 
> I’d call this finishCreatingPasteboard.
> 
Done

> > Source/WebKit/win/WebCoreSupport/WebDragClient.cpp:34
> > +#if !USE(LEGACY_STYLE_ABSTRACT_CLIPBOARD_CLASS)
> 
> Should not use this #if in this file, since it’s only used for PLATFORM(WIN). Should just keep the !USE(LEGACY_STYLE_ABSTRACT_CLIPBOARD_CLASS) and delete the other side.
> 
Done

> > Source/WebKit/win/WebCoreSupport/WebDragClient.cpp:117
> > +    Pasteboard* pasteboard = const_cast<Pasteboard*>(&clipboard->pasteboard());
> > +    COMPtr<IDataObject> dataObject = pasteboard->dataObject();
> 
> Doesn’t make sense that you have to const_cast here. Can’t dataObject be a const member function?
Done

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