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

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=133782

Gyuyoung Kim <gyuyoung.kim at webkit.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #240828|review?                     |review-
              Flags|                            |

--- Comment #15 from Gyuyoung Kim <gyuyoung.kim at webkit.org> ---
Comment on attachment 240828
  --> https://bugs.webkit.org/attachment.cgi?id=240828
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.

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-unassigned/attachments/20141103/70713276/attachment-0002.html>