Summary: | [WK2][EFL] Implement text selection range and url's copy and paste through EFL MiniBrowser Context Menu | ||||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Joonghun Park <jh718.park> | ||||||||||||||||||||||||||||||
Component: | WebKit EFL | Assignee: | Joonghun Park <jh718.park> | ||||||||||||||||||||||||||||||
Status: | RESOLVED WONTFIX | ||||||||||||||||||||||||||||||||
Severity: | Normal | CC: | buildbot, cdumez, cgarcia, gyuyoung.kim, hh.kaka, lucas.de.marchi, mcatanzaro, mrobinson, rniwa | ||||||||||||||||||||||||||||||
Priority: | P2 | ||||||||||||||||||||||||||||||||
Version: | 528+ (Nightly build) | ||||||||||||||||||||||||||||||||
Hardware: | Unspecified | ||||||||||||||||||||||||||||||||
OS: | Unspecified | ||||||||||||||||||||||||||||||||
Attachments: |
|
Description
Joonghun Park
2014-06-11 21:19:51 PDT
Created attachment 232929 [details]
Patch
Created attachment 233031 [details]
Patch
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 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
Created attachment 233294 [details]
Patch
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 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
Created attachment 233351 [details]
Patch
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. Created attachment 234485 [details]
Patch
Created attachment 240823 [details]
<WIP>
FYI, the correct way I believe is to remove the review flag, not r- your own patches. (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~ :) Created attachment 240828 [details]
Patch
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. Created attachment 263674 [details]
Patch
Created attachment 263678 [details]
Patch
Created attachment 263800 [details]
Use PasteboardHelper instead of Efl specific clipboard temporarily
Created attachment 263803 [details]
Remove redundant std::unique_ptr<Pasteboard> from Editor.h
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. (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. Created attachment 263913 [details]
Use BSD license in added files
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.
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. |