Bug 133782 – [WK2][EFL] Implement text selection range and url's copy and paste through EFL MiniBrowser Context Menu

• Status: RESOLVED
• Resolution: WONTFIX
• Component: WebKit EFL

[Joonghun Park]

By adding dataobjectEfl and ArgumentCodersEfl, text selection range and url data transmission between uiprocess and webprocess is supported.
These are the base part for implementation of Drag & Drop in EFL port, also.

[Joonghun Park]

Created attachment 232929 [details]
Patch

[Joonghun Park]

Created attachment 233031 [details]
Patch

[Build Bot]

Comment on attachment 233031 [details]
Patch

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

New failing tests:
media/W3C/video/networkState/networkState_during_loadstart.html

[Build Bot]

Created attachment 233038 [details]
Archive of layout-test-results from webkit-ews-12 for mac-mountainlion-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: webkit-ews-12  Port: mac-mountainlion-wk2  Platform: Mac OS X 10.8.5

[Joonghun Park]

Created attachment 233294 [details]
Patch

[Build Bot]

Comment on attachment 233294 [details]
Patch

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

New failing tests:
media/W3C/video/networkState/networkState_during_loadstart.html

[Build Bot]

Created attachment 233315 [details]
Archive of layout-test-results from webkit-ews-09 for mac-mountainlion-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: webkit-ews-09  Port: mac-mountainlion-wk2  Platform: Mac OS X 10.8.5

[Joonghun Park]

Created attachment 233351 [details]
Patch

[Gyuyoung Kim]

Comment on attachment 233351 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=233351&action=review

This patch is almost copied from GTK implementation. So, please consider my suggestion. So, r-

> Source/WebCore/ChangeLog:4
> +        Need the bug URL (OOPS!).

Remove this template.

> Source/WebCore/ChangeLog:7
> +

Missing patch description.

> Source/WebCore/platform/efl/DataObjectEfl.cpp:4
> +    This library is free software; you can redistribute it and/or

Use BSD instead of LGPL license.

> Source/WebCore/platform/efl/DataObjectEfl.h:20
> +#ifndef DataObjectEfl_h

It looks DataObjectGtk.h/cpp files are almost same with DataObjectGtk.h/cpp. In this case, you have to add *original author* in Licence. Besides I think we can share common functions with GTK port. So, we may make DataObject.h/cpp to be shared by GTK and EFL. Then, we make DataObjectGtk.h/cpp and DataObjectEfl.h/cpp to support port specific implementation.

[Joonghun Park]

Created attachment 234485 [details]
Patch

[Joonghun Park]

Created attachment 240823 [details]
<WIP>

[Chris Dumez]

FYI, the correct way I believe is to remove the review flag, not r- your own patches.

[Joonghun Park]

(In reply to comment #12)
> FYI, the correct way I believe is to remove the review flag, not r- your own
> patches.

I revised review flag according to your advice. Thank you for your kindness~ :)

[Joonghun Park]

Created attachment 240828 [details]
Patch

[Gyuyoung Kim]

Comment on attachment 240828 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=240828&action=review

I think this patch needs to be reviewed further. I add GTK reviewers as well.

> Source/WebCore/editing/Editor.cpp:1312
> +        m_pasteboard = Pasteboard::createForCopyAndPaste();

We should not have EFL specific code path as much as possible. In this case, it looks we can use exist code path. Why do we need to have own code path ?

> Source/WebCore/editing/Editor.cpp:1333
> +            m_pasteboard = Pasteboard::createForCopyAndPaste();

Why should EFL port take different path ? Can't we implement writeSelectionToPasteboard() in EditorEfl.cpp ?

void Editor::writeSelectionToPasteboard(Pasteboard&)
{
    notImplemented();
}

> Source/WebCore/editing/Editor.cpp:1347
> +#if PLATFORM(EFL)

It looks it would be nicer to implement EFL specific function in EditorEfl.cpp. To do that, you need to add EditorEfl.h and declare needed function to virtual.

> Source/WebCore/editing/Editor.h:536
> +    OwnPtr<Pasteboard> m_pasteboard; // FIXME: This is temporary. It should be removed by using system clipboard.

Use std::unique_ptr<>

> Source/WebCore/platform/DataObject.h:52
> +    bool hasImage() const { return !m_image.isEmpty(); }

bool hasImage() should be declared as virtual. Then own DataObject file should override this function.

> Source/WebCore/platform/DataObject.h:59
> +    void clearImage() { m_image = ""; }

ditto.

> Source/WebCore/platform/efl/DataObjectEfl.cpp:23
> +#if PLATFORM(EFL)

Looks unneeded guard.

[Joonghun Park]

Created attachment 263674 [details]
Patch

[Joonghun Park]

Created attachment 263678 [details]
Patch

[Joonghun Park]

Created attachment 263800 [details]
Use PasteboardHelper instead of Efl specific clipboard temporarily

[Joonghun Park]

Created attachment 263803 [details]
Remove redundant std::unique_ptr<Pasteboard> from Editor.h

[Gyuyoung Kim]

Comment on attachment 263803 [details]
Remove redundant std::unique_ptr<Pasteboard> from Editor.h

View in context: https://bugs.webkit.org/attachment.cgi?id=263803&action=review

Basically this change should be granted by GTK reviewers. Then we can go ahead to review. Please talk with GTK reviewer first. BTW is there any tests for copy & paste ?

> Source/WebCore/platform/efl/PasteboardHelper.h:4
> +    This library is free software; you can redistribute it and/or

Use BSD license.

[Joonghun Park]

(In reply to comment #20)
> Comment on attachment 263803 [details]
> Remove redundant std::unique_ptr<Pasteboard> from Editor.h
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=263803&action=review
> 
> Basically this change should be granted by GTK reviewers. Then we can go
> ahead to review. Please talk with GTK reviewer first. BTW is there any tests
> for copy & paste ?
> 
> > Source/WebCore/platform/efl/PasteboardHelper.h:4
> > +    This library is free software; you can redistribute it and/or
> 
> Use BSD license.

Ok, I got it. Regarding the tests, let me check for it.
For now, I will change the license as you commented.

[Joonghun Park]

Created attachment 263913 [details]
Use BSD license in added files

[Alex Christensen]

Comment on attachment 263913 [details]
Use BSD license in added files

EFL port was removed.  If this refactoring is still helpful for maintaining a downstream port, please upload a patch that does the refactoring without adding EFL code.

[Michael Catanzaro]

Closing this bug because the EFL port has been removed from trunk.

If you feel this bug applies to a different upstream WebKit port and was closed in error, please either update the title and reopen the bug, or leave a comment to request this.