[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 bugzilla-daemon at webkit.org
Mon Nov 3 01:56:05 PST 2014


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

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&)

> Source/WebCore/editing/Editor.cpp:1347

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 = ""; }


> Source/WebCore/platform/efl/DataObjectEfl.cpp:23

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>

More information about the webkit-unassigned mailing list