Bug 119949 – Factor Clipboard into drag and non-drag parts

• Status: RESOLVED
• Resolution: FIXED
• Component: DOM

[Darin Adler]

Factor Clipboard into drag and non-drag parts

[Darin Adler]

Created attachment 208998 [details]
Patch

[Early Warning System Bot]

Comment on attachment 208998 [details]
Patch

Attachment 208998 [details] did not pass qt-ews (qt):
Output: http://webkit-queues.appspot.com/results/1465996

[Early Warning System Bot]

Comment on attachment 208998 [details]
Patch

Attachment 208998 [details] did not pass qt-wk2-ews (qt-wk2):
Output: http://webkit-queues.appspot.com/results/1493097

[EFL EWS Bot]

Comment on attachment 208998 [details]
Patch

Attachment 208998 [details] did not pass efl-ews (efl):
Output: http://webkit-queues.appspot.com/results/1433573

[EFL EWS Bot]

Comment on attachment 208998 [details]
Patch

Attachment 208998 [details] did not pass efl-wk2-ews (efl-wk2):
Output: http://webkit-queues.appspot.com/results/1465995

[Build Bot]

Comment on attachment 208998 [details]
Patch

Attachment 208998 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.appspot.com/results/1498129

[kov's GTK+ EWS bot]

Comment on attachment 208998 [details]
Patch

Attachment 208998 [details] did not pass gtk-ews (gtk):
Output: http://webkit-queues.appspot.com/results/1493098

[Darin Adler]

Created attachment 209000 [details]
Patch

[Darin Adler]

Committed r154260: <http://trac.webkit.org/changeset/154260>

[Daniel Bates]

(In reply to comment #9)
> Committed r154260: <http://trac.webkit.org/changeset/154260>

This change breaks all builds that disable drag support, such as the iOS WebKit build (*), because we only declare Clipboard::hasData() when building with drag support enabled, but always define Clipboard::hasData() (since its implementation isn't surrounded by a "ENABLE(DRAG_SUPPORT)" guard).

I landed a build fix that defines Clipboard::hasData() when building with and without drag support in <http://trac.webkit.org/changeset/154350>.

Although Clipboard::hasData() is only used in DragController, I chose this approach as opposed to adding an ENABLE(DRAG_SUPPORT) guard around the implementation of Clipboard::hasData() (and the declaration and definition for the Pasteboard method of the same name) because Clipboard exposes other methods for interacting with the underlying pasteboard data when building without drag support, including Clipboard::{clear, get, set}Data() and it seemed strange to segregate hasData() from these other methods based on the availability of drag support. Let me know if we should segregate hasData() from the other Clipboard::*Data methods or if there is a better approach to fixing the build for ports that don't enable drag support.

(*) For completeness, the following is the error we get when we build the iOS port with this change:

Source/WebCore/dom/Clipboard.cpp:111:17: error: out-of-line definition of 'hasData' does not match any declaration in 'WebCore::Clipboard'

[Darin Adler]

(In reply to comment #10)
> Although Clipboard::hasData() is only used in DragController, I chose this approach as opposed to adding an ENABLE(DRAG_SUPPORT) guard around the implementation of Clipboard::hasData() (and the declaration and definition for the Pasteboard method of the same name) because Clipboard exposes other methods for interacting with the underlying pasteboard data when building without drag support

I would have done it the other way, fixing the .cpp file and putting the hasData function in the section for drag support. I am trying to thin out this class. I don’t want it to have lots of functions for logical completeness; I want it to be as small as possible.

[Daniel Bates]

(In reply to comment #11)
> (In reply to comment #10)
> > Although Clipboard::hasData() is only used in DragController, I chose this approach as opposed to adding an ENABLE(DRAG_SUPPORT) guard around the implementation of Clipboard::hasData() (and the declaration and definition for the Pasteboard method of the same name) because Clipboard exposes other methods for interacting with the underlying pasteboard data when building without drag support
> 
> I would have done it the other way, fixing the .cpp file and putting the hasData function in the section for drag support. I am trying to thin out this class. I don’t want it to have lots of functions for logical completeness; I want it to be as small as possible.

Filed bug #120088 to only declare and define Clipboard::hasData() when building with drag support.