Merge ~osomon/oxide:contextmenu-copyImage into oxide:master

Proposed by Olivier Tilloy
Status: Merged
Merged at revision: 1776161f8f04b5921020843f899f9eb8e105b8db
Proposed branch: ~osomon/oxide:contextmenu-copyImage
Merge into: oxide:master
Diff against target: 284 lines (+64/-38)
11 files modified
qt/core/browser/clipboard/oxide_qt_clipboard.cc (+4/-27)
qt/core/browser/oxide_qt_skutils.cc (+8/-4)
qt/core/browser/oxide_qt_web_context_menu.cc (+5/-1)
qt/core/browser/oxide_qt_web_context_menu.h (+2/-1)
qt/core/glue/oxide_qt_web_context_menu_proxy_client.h (+2/-1)
qt/quick/oxide_qquick_web_context_menu.cc (+18/-1)
qt/tests/qmltests/api/tst_WebView_contextMenu.qml (+8/-0)
qt/tests/qmltests/qml_test_support.cc (+5/-1)
qt/tests/qmltests/qml_test_support.h (+2/-0)
shared/browser/oxide_web_context_menu.cc (+8/-1)
shared/browser/oxide_web_context_menu.h (+2/-1)
Reviewer Review Type Date Requested Status
Chris Coulson Approve
Review via email: mp+295690@code.launchpad.net

Commit message

Add a copyImage() method to the context menu to copy the contents of an image element to the clipboard. Fixes https://launchpad.net/bugs/1585291.

To post a comment you must log in.
Revision history for this message
Chris Coulson (chrisccoulson) wrote :

Thanks, I've left a few small comments inline

review: Needs Fixing
~osomon/oxide:contextmenu-copyImage updated
9c5022d... by Olivier Tilloy

Specify fully-qualified include path.

e442309... by Olivier Tilloy

Detach image instead of copying it.

8a323c1... by Olivier Tilloy

Call detach() on the returned image in QImageFromSkBitmap().

d420348... by Olivier Tilloy

Re-arrange code for improved readability.

Revision history for this message
Olivier Tilloy (osomon) wrote :

Updated.

Revision history for this message
Chris Coulson (chrisccoulson) :
review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
diff --git a/qt/core/browser/clipboard/oxide_qt_clipboard.cc b/qt/core/browser/clipboard/oxide_qt_clipboard.cc
index d6fd870..38d003e 100644
--- a/qt/core/browser/clipboard/oxide_qt_clipboard.cc
+++ b/qt/core/browser/clipboard/oxide_qt_clipboard.cc
@@ -1,5 +1,5 @@
1// vim:expandtab:shiftwidth=2:tabstop=2:1// vim:expandtab:shiftwidth=2:tabstop=2:
2// Copyright (C) 2015 Canonical Ltd.2// Copyright (C) 2015-2016 Canonical Ltd.
33
4// This library is free software; you can redistribute it and/or4// This library is free software; you can redistribute it and/or
5// modify it under the terms of the GNU Lesser General Public5// modify it under the terms of the GNU Lesser General Public
@@ -30,6 +30,8 @@
30#include <QObject>30#include <QObject>
31#include <QString>31#include <QString>
3232
33#include "qt/core/browser/oxide_qt_skutils.h"
34
33#define GET_CLIPBOARD_DATA(c) \35#define GET_CLIPBOARD_DATA(c) \
34 c->mimeData( \36 c->mimeData( \
35 type == ui::CLIPBOARD_TYPE_COPY_PASTE ? \37 type == ui::CLIPBOARD_TYPE_COPY_PASTE ? \
@@ -300,32 +302,7 @@ void Clipboard::WriteWebSmartPaste() {
300302
301void Clipboard::WriteBitmap(const SkBitmap& bitmap) {303void Clipboard::WriteBitmap(const SkBitmap& bitmap) {
302 DCHECK(CalledOnValidThread());304 DCHECK(CalledOnValidThread());
303 QImage image;305 write_mime_data_acc_->setImageData(QImageFromSkBitmap(bitmap));
304 if (bitmap.info().colorType() != kN32_SkColorType) {
305 SkImageInfo info =
306 SkImageInfo::MakeN32(bitmap.width(),
307 bitmap.height(),
308 bitmap.alphaType());
309
310 SkBitmap convertedBitmap;
311 if (!convertedBitmap.tryAllocPixels(info)) {
312 return;
313 }
314
315 bitmap.readPixels(info, convertedBitmap.getPixels(), 0, 0, 0);
316
317 image = QImage(reinterpret_cast<const uchar *>(convertedBitmap.getPixels()),
318 bitmap.width(),
319 bitmap.height(),
320 QImage::Format_RGBA8888);
321 } else {
322 image = QImage(reinterpret_cast<const uchar *>(bitmap.getPixels()),
323 bitmap.width(),
324 bitmap.height(),
325 QImage::Format_RGBA8888);
326 }
327
328 write_mime_data_acc_->setImageData(image.copy());
329}306}
330307
331void Clipboard::WriteData(const FormatType& format,308void Clipboard::WriteData(const FormatType& format,
diff --git a/qt/core/browser/oxide_qt_skutils.cc b/qt/core/browser/oxide_qt_skutils.cc
index d7bc690..0aecca5 100644
--- a/qt/core/browser/oxide_qt_skutils.cc
+++ b/qt/core/browser/oxide_qt_skutils.cc
@@ -1,5 +1,5 @@
1// vim:expandtab:shiftwidth=2:tabstop=2:1// vim:expandtab:shiftwidth=2:tabstop=2:
2// Copyright (C) 2014 Canonical Ltd.2// Copyright (C) 2014-2016 Canonical Ltd.
33
4// This library is free software; you can redistribute it and/or4// This library is free software; you can redistribute it and/or
5// modify it under the terms of the GNU Lesser General Public5// modify it under the terms of the GNU Lesser General Public
@@ -102,12 +102,16 @@ QImage QImageFromSkBitmap(const SkBitmap& bitmap) {
102 QImageFormatFromSkColorType(color, alpha == kPremul_SkAlphaType);102 QImageFormatFromSkColorType(color, alpha == kPremul_SkAlphaType);
103 if (format != QImage::Format_Invalid) {103 if (format != QImage::Format_Invalid) {
104 SkAutoLockPixels lock(bitmap);104 SkAutoLockPixels lock(bitmap);
105 return QImage(reinterpret_cast<const uchar*>(bitmap.getPixels()),105 QImage image(reinterpret_cast<const uchar*>(bitmap.getPixels()),
106 bitmap.width(), bitmap.height(), format);106 bitmap.width(), bitmap.height(), format);
107 image.detach();
108 return image;
107 }109 }
108110
109 if (color == kBGRA_8888_SkColorType) {111 if (color == kBGRA_8888_SkColorType) {
110 return QImageFromSkBitmap_BGRA_8888(bitmap);112 QImage image = QImageFromSkBitmap_BGRA_8888(bitmap);
113 image.detach();
114 return image;
111 }115 }
112116
113 LOG(WARNING) <<117 LOG(WARNING) <<
diff --git a/qt/core/browser/oxide_qt_web_context_menu.cc b/qt/core/browser/oxide_qt_web_context_menu.cc
index 29202e2..acbbeae 100644
--- a/qt/core/browser/oxide_qt_web_context_menu.cc
+++ b/qt/core/browser/oxide_qt_web_context_menu.cc
@@ -1,5 +1,5 @@
1// vim:expandtab:shiftwidth=2:tabstop=2:1// vim:expandtab:shiftwidth=2:tabstop=2:
2// Copyright (C) 2015 Canonical Ltd.2// Copyright (C) 2015-2016 Canonical Ltd.
33
4// This library is free software; you can redistribute it and/or4// This library is free software; you can redistribute it and/or
5// modify it under the terms of the GNU Lesser General Public5// modify it under the terms of the GNU Lesser General Public
@@ -169,6 +169,10 @@ int WebContextMenu::mediaFlags() const {
169 return flags;169 return flags;
170}170}
171171
172void WebContextMenu::copyImage() const {
173 CopyImage();
174}
175
172void WebContextMenu::saveLink() const {176void WebContextMenu::saveLink() const {
173 SaveLink();177 SaveLink();
174}178}
diff --git a/qt/core/browser/oxide_qt_web_context_menu.h b/qt/core/browser/oxide_qt_web_context_menu.h
index 8db72d8..257db2b 100644
--- a/qt/core/browser/oxide_qt_web_context_menu.h
+++ b/qt/core/browser/oxide_qt_web_context_menu.h
@@ -1,5 +1,5 @@
1// vim:expandtab:shiftwidth=2:tabstop=2:1// vim:expandtab:shiftwidth=2:tabstop=2:
2// Copyright (C) 2015 Canonical Ltd.2// Copyright (C) 2015-2016 Canonical Ltd.
33
4// This library is free software; you can redistribute it and/or4// This library is free software; you can redistribute it and/or
5// modify it under the terms of the GNU Lesser General Public5// modify it under the terms of the GNU Lesser General Public
@@ -60,6 +60,7 @@ class WebContextMenu : public oxide::WebContextMenu,
60 void cancel() override;60 void cancel() override;
61 int editFlags() const override;61 int editFlags() const override;
62 int mediaFlags() const override;62 int mediaFlags() const override;
63 void copyImage() const override;
63 void saveLink() const override;64 void saveLink() const override;
64 void saveMedia() const override;65 void saveMedia() const override;
6566
diff --git a/qt/core/glue/oxide_qt_web_context_menu_proxy_client.h b/qt/core/glue/oxide_qt_web_context_menu_proxy_client.h
index b5d0131..123297e 100644
--- a/qt/core/glue/oxide_qt_web_context_menu_proxy_client.h
+++ b/qt/core/glue/oxide_qt_web_context_menu_proxy_client.h
@@ -1,5 +1,5 @@
1// vim:expandtab:shiftwidth=2:tabstop=2:1// vim:expandtab:shiftwidth=2:tabstop=2:
2// Copyright (C) 2015 Canonical Ltd.2// Copyright (C) 2015-2016 Canonical Ltd.
33
4// This library is free software; you can redistribute it and/or4// This library is free software; you can redistribute it and/or
5// modify it under the terms of the GNU Lesser General Public5// modify it under the terms of the GNU Lesser General Public
@@ -71,6 +71,7 @@ class WebContextMenuProxyClient {
7171
72 virtual int mediaFlags() const = 0;72 virtual int mediaFlags() const = 0;
7373
74 virtual void copyImage() const = 0;
74 virtual void saveLink() const = 0;75 virtual void saveLink() const = 0;
75 virtual void saveMedia() const = 0;76 virtual void saveMedia() const = 0;
76};77};
diff --git a/qt/quick/oxide_qquick_web_context_menu.cc b/qt/quick/oxide_qquick_web_context_menu.cc
index 31b4575..561f421 100644
--- a/qt/quick/oxide_qquick_web_context_menu.cc
+++ b/qt/quick/oxide_qquick_web_context_menu.cc
@@ -1,5 +1,5 @@
1// vim:expandtab:shiftwidth=2:tabstop=2:1// vim:expandtab:shiftwidth=2:tabstop=2:
2// Copyright (C) 2015 Canonical Ltd.2// Copyright (C) 2015-2016 Canonical Ltd.
33
4// This library is free software; you can redistribute it and/or4// This library is free software; you can redistribute it and/or
5// modify it under the terms of the GNU Lesser General Public5// modify it under the terms of the GNU Lesser General Public
@@ -71,6 +71,7 @@ class ContextMenuContext : public QObject {
71 OxideQQuickWebView::EditCapabilities editFlags() const;71 OxideQQuickWebView::EditCapabilities editFlags() const;
72 OxideQQuickWebView::MediaStatus mediaFlags() const;72 OxideQQuickWebView::MediaStatus mediaFlags() const;
7373
74 Q_INVOKABLE void copyImage() const;
74 Q_INVOKABLE void saveLink() const;75 Q_INVOKABLE void saveLink() const;
75 Q_INVOKABLE void saveMedia() const;76 Q_INVOKABLE void saveMedia() const;
7677
@@ -214,6 +215,22 @@ OxideQQuickWebView::MediaStatus ContextMenuContext::mediaFlags() const {
214 return static_cast<OxideQQuickWebView::MediaStatus>(client_->mediaFlags());215 return static_cast<OxideQQuickWebView::MediaStatus>(client_->mediaFlags());
215}216}
216217
218void ContextMenuContext::copyImage() const {
219 OxideQQuickWebView::MediaType media_type = mediaType();
220 if (media_type != OxideQQuickWebView::MediaTypeImage &&
221 media_type != OxideQQuickWebView::MediaTypeCanvas) {
222 qWarning() << "ContextMenuContext::copyImage(): not an image";
223 return;
224 }
225
226 if (!hasImageContents()) {
227 qWarning() << "ContextMenuContext::copyImage(): image has no contents";
228 return;
229 }
230
231 client_->copyImage();
232}
233
217void ContextMenuContext::saveLink() const {234void ContextMenuContext::saveLink() const {
218 if (linkUrl().isValid()) {235 if (linkUrl().isValid()) {
219 client_->saveLink();236 client_->saveLink();
diff --git a/qt/tests/qmltests/api/tst_WebView_contextMenu.qml b/qt/tests/qmltests/api/tst_WebView_contextMenu.qml
index 6017a1b..f8954b2 100644
--- a/qt/tests/qmltests/api/tst_WebView_contextMenu.qml
+++ b/qt/tests/qmltests/api/tst_WebView_contextMenu.qml
@@ -96,6 +96,14 @@ TestWebView {
96 compare(model.isEditable, data.isEditable);96 compare(model.isEditable, data.isEditable);
97 }97 }
9898
99 function test_WebView_contextMenu_copyImage() {
100 ClipboardTestUtils.clearClipboard();
101 verify(!ClipboardTestUtils.hasImage());
102 invokeContextMenu("image");
103 webView.currentContextMenu.contextModel.copyImage();
104 verify(TestUtils.waitFor(ClipboardTestUtils.hasImage));
105 }
106
99 function test_WebView_contextMenu_saveLink() {107 function test_WebView_contextMenu_saveLink() {
100 invokeContextMenu("hyperlink");108 invokeContextMenu("hyperlink");
101 var model = webView.currentContextMenu.contextModel;109 var model = webView.currentContextMenu.contextModel;
diff --git a/qt/tests/qmltests/qml_test_support.cc b/qt/tests/qmltests/qml_test_support.cc
index 4c464c0..1de24a5 100644
--- a/qt/tests/qmltests/qml_test_support.cc
+++ b/qt/tests/qmltests/qml_test_support.cc
@@ -1,5 +1,5 @@
1// vim:expandtab:shiftwidth=2:tabstop=2:1// vim:expandtab:shiftwidth=2:tabstop=2:
2// Copyright (C) 2013-2015 Canonical Ltd.2// Copyright (C) 2013-2016 Canonical Ltd.
33
4// This library is free software; you can redistribute it and/or4// This library is free software; you can redistribute it and/or
5// modify it under the terms of the GNU Lesser General Public5// modify it under the terms of the GNU Lesser General Public
@@ -68,6 +68,10 @@ void ExternalProtocolHandler::setScheme(const QString& scheme) {
6868
69ClipboardTestUtils::ClipboardTestUtils() {}69ClipboardTestUtils::ClipboardTestUtils() {}
7070
71bool ClipboardTestUtils::hasImage() const {
72 return QGuiApplication::clipboard()->mimeData()->hasImage();
73}
74
71void ClipboardTestUtils::copyToClipboard(const QString& mimeType,75void ClipboardTestUtils::copyToClipboard(const QString& mimeType,
72 const QString& data) {76 const QString& data) {
73 QMimeData * mime_data = new QMimeData();77 QMimeData * mime_data = new QMimeData();
diff --git a/qt/tests/qmltests/qml_test_support.h b/qt/tests/qmltests/qml_test_support.h
index 8d400bd..ac0f904 100644
--- a/qt/tests/qmltests/qml_test_support.h
+++ b/qt/tests/qmltests/qml_test_support.h
@@ -58,6 +58,8 @@ class ClipboardTestUtils : public QObject {
58 public:58 public:
59 ClipboardTestUtils();59 ClipboardTestUtils();
6060
61 Q_INVOKABLE bool hasImage() const;
62
61 Q_INVOKABLE void copyToClipboard(const QString& mimeType,63 Q_INVOKABLE void copyToClipboard(const QString& mimeType,
62 const QString& data);64 const QString& data);
6365
diff --git a/shared/browser/oxide_web_context_menu.cc b/shared/browser/oxide_web_context_menu.cc
index 718b7f9..6efa9f2 100644
--- a/shared/browser/oxide_web_context_menu.cc
+++ b/shared/browser/oxide_web_context_menu.cc
@@ -1,5 +1,5 @@
1// vim:expandtab:shiftwidth=2:tabstop=2:1// vim:expandtab:shiftwidth=2:tabstop=2:
2// Copyright (C) 2015 Canonical Ltd.2// Copyright (C) 2015-2016 Canonical Ltd.
33
4// This library is free software; you can redistribute it and/or4// This library is free software; you can redistribute it and/or
5// modify it under the terms of the GNU Lesser General Public5// modify it under the terms of the GNU Lesser General Public
@@ -79,6 +79,13 @@ void WebContextMenu::Close() {
79 content::BrowserThread::UI, FROM_HERE, this);79 content::BrowserThread::UI, FROM_HERE, this);
80}80}
8181
82void WebContextMenu::CopyImage() const {
83 if ((params_.media_type == blink::WebContextMenuData::MediaTypeCanvas) ||
84 (params_.media_type == blink::WebContextMenuData::MediaTypeImage)) {
85 render_frame_host_->GetRenderViewHost()->CopyImageAt(params_.x, params_.y);
86 }
87}
88
82void WebContextMenu::SaveLink() const {89void WebContextMenu::SaveLink() const {
83 content::BrowserContext* context = web_contents()->GetBrowserContext();90 content::BrowserContext* context = web_contents()->GetBrowserContext();
84 content::DownloadManager* dlm =91 content::DownloadManager* dlm =
diff --git a/shared/browser/oxide_web_context_menu.h b/shared/browser/oxide_web_context_menu.h
index 24f9277..8ca6d35 100644
--- a/shared/browser/oxide_web_context_menu.h
+++ b/shared/browser/oxide_web_context_menu.h
@@ -1,5 +1,5 @@
1// vim:expandtab:shiftwidth=2:tabstop=2:1// vim:expandtab:shiftwidth=2:tabstop=2:
2// Copyright (C) 2015 Canonical Ltd.2// Copyright (C) 2015-2016 Canonical Ltd.
33
4// This library is free software; you can redistribute it and/or4// This library is free software; you can redistribute it and/or
5// modify it under the terms of the GNU Lesser General Public5// modify it under the terms of the GNU Lesser General Public
@@ -56,6 +56,7 @@ class OXIDE_SHARED_EXPORT WebContextMenu : public content::WebContentsObserver {
5656
57 content::ContextMenuParams params_;57 content::ContextMenuParams params_;
5858
59 void CopyImage() const;
59 void SaveLink() const;60 void SaveLink() const;
60 void SaveMedia() const;61 void SaveMedia() const;
6162

Subscribers

People subscribed via source and target branches