Bug 120088 – Define Clipboard::hasData() only when building with drag support

• Status: RESOLVED
• Resolution: FIXED
• Component: WebCore Misc.

[Daniel Bates]

Following up from Darin Adler's remark in comment 11 bug #119949, define Clipboard::hasData() only when building with drag support enabled.

[Daniel Bates]

Created attachment 209229 [details]
Patch

[Daniel Bates]

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

[Daniel Bates]

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.

[Darin Adler]

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.

[Daniel Bates]

(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.

[Daniel Bates]

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

[Darin Adler]

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!