Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

browser(webkit): drag support for wpe and gtk #4639

Merged
merged 1 commit into from Dec 23, 2020

Conversation

JoelEinbinder
Copy link
Contributor

@JoelEinbinder JoelEinbinder commented Dec 8, 2020

mac support coming next, followed by windows.

JoelEinbinder/webkit@036ee12

+++ b/Source/WebCore/page/wpe/DragControllerWPE.cpp
@@ -0,0 +1,79 @@
+/*
+ * Copyright (C) 2007-20 Apple Inc. All rights reserved.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It might be easier to rename DragControllerGtk.cpp to DragControllerGlib.cpp and use it in both linux builds. This was they wouldn't diverge at least. But up to you.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wasn't able to get it to easily compile without a much larger diff. I am not super worried about this code diverging, because we would probably rather match the mac behavior than whatever gtk decides to do. I'll try a follow up patch to see how it looks though.


#if PLATFORM(GTK)
explicit Pasteboard(const String& name);
- explicit Pasteboard(SelectionData&);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

revert this line?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

String Pasteboard::readString(const String& type)
{
+ if (m_selectionData)
+ return m_selectionData->text();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we care about other selection data types?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, fixed!


- dragData = DragData(pasteboardName, clientPosition, globalPosition, draggingSourceOperationMask, applicationFlags, dragDestinationActionMask);
+ dragData = DragData(firstArg, clientPosition, globalPosition, draggingSourceOperationMask, applicationFlags, dragDestinationActionMask);
+#if PLATFORM(COCOA)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't it used on Windows too?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Windows doesn't have drag & drop support in webkit2, so it never gets used. My follow up patch adds windows support.

- String pasteboardName;
- Vector<String> fileNames;
+#if PLATFORM(WPE)
+ SelectionData* selectionData = new SelectionData();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Where is it deleted? Consider converting it into a smart pointer.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I couldn't get a smart pointer working, but I refactored this so wpe works the same as gtk and doesn't need this code.

+ // We intercept any drags generated by this mouse event
+ // to prevent them from creating actual drags in the host
+ // operating system.
+ // So far this is only implemented for WPE.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

does GTK code not work (I see it is enabled)?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

stale comment, fixed!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants