Bug 120088

Created attachment 209229 [details] Patch

Should we also add a ENABLE(DRAG_SUPPORT) guard around Pasteboard::hasData()?

Would it be better to move the definition of hasData() to another place in the file so as to avoid mixing it with the Clipboard IDL functions and/or to more closely match its position in Clipboard.h? Having said that, I like that hasData() is grouped with logically related functions.

Comment on attachment 209229 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=209229&action=review > Source/WebCore/dom/Clipboard.cpp:116 > +#if ENABLE(DRAG_SUPPORT) > bool Clipboard::hasData() > { > return m_pasteboard->hasData(); > } > +#endif I suggest moving it down to the DRAG_SUPPORT part of the file. I chose that over littering the file with #if statements. Note that this function does *not* belong in the same family as functions like getData; if it was suitable for exposure to the web as getData is, it would have to check canReadData and return false unconditionally in that case. It’s really a function just for internal use. > Source/WebCore/dom/Clipboard.h:85 > static PassRefPtr<Clipboard> createForDragAndDrop(); > static PassRefPtr<Clipboard> createForDragAndDrop(ClipboardAccessPolicy, const DragData&); > + bool hasData(); Should not be grouped with the create functions in a single paragraph. Just include another blank line, please.

(In reply to comment #4) > > Source/WebCore/dom/Clipboard.cpp:116 > > +#if ENABLE(DRAG_SUPPORT) > > bool Clipboard::hasData() > > { > > return m_pasteboard->hasData(); > > } > > +#endif > > I suggest moving it down to the DRAG_SUPPORT part of the file. I chose that over littering the file with #if statements. > [...] Will move definition to the DRAG_SUPPORT part of the file. > > Source/WebCore/dom/Clipboard.h:85 > > static PassRefPtr<Clipboard> createForDragAndDrop(); > > static PassRefPtr<Clipboard> createForDragAndDrop(ClipboardAccessPolicy, const DragData&); > > + bool hasData(); > > Should not be grouped with the create functions in a single paragraph. Just include another blank line, please. Will add empty line above hasData() declaration.

Committed r154360: <http://trac.webkit.org/changeset/154360>

After you landed this, I figured out what I really want to do (and will do soon). Just have the caller get at the pasteboard directly instead of having this forwarding function. So I’ll be removing this function entirely. Thanks!