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

Proposed by Olivier Tilloy on 2016-05-25
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 2016-05-25 Approve on 2016-06-16
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.
Chris Coulson (chrisccoulson) wrote :

Thanks, I've left a few small comments inline

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

Specify fully-qualified include path.

e442309... by Olivier Tilloy on 2016-06-15

Detach image instead of copying it.

8a323c1... by Olivier Tilloy on 2016-06-15

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

d420348... by Olivier Tilloy on 2016-06-15

Re-arrange code for improved readability.

Olivier Tilloy (osomon) wrote :

Updated.

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1diff --git a/qt/core/browser/clipboard/oxide_qt_clipboard.cc b/qt/core/browser/clipboard/oxide_qt_clipboard.cc
2index d6fd870..38d003e 100644
3--- a/qt/core/browser/clipboard/oxide_qt_clipboard.cc
4+++ b/qt/core/browser/clipboard/oxide_qt_clipboard.cc
5@@ -1,5 +1,5 @@
6 // vim:expandtab:shiftwidth=2:tabstop=2:
7-// Copyright (C) 2015 Canonical Ltd.
8+// Copyright (C) 2015-2016 Canonical Ltd.
9
10 // This library is free software; you can redistribute it and/or
11 // modify it under the terms of the GNU Lesser General Public
12@@ -30,6 +30,8 @@
13 #include <QObject>
14 #include <QString>
15
16+#include "qt/core/browser/oxide_qt_skutils.h"
17+
18 #define GET_CLIPBOARD_DATA(c) \
19 c->mimeData( \
20 type == ui::CLIPBOARD_TYPE_COPY_PASTE ? \
21@@ -300,32 +302,7 @@ void Clipboard::WriteWebSmartPaste() {
22
23 void Clipboard::WriteBitmap(const SkBitmap& bitmap) {
24 DCHECK(CalledOnValidThread());
25- QImage image;
26- if (bitmap.info().colorType() != kN32_SkColorType) {
27- SkImageInfo info =
28- SkImageInfo::MakeN32(bitmap.width(),
29- bitmap.height(),
30- bitmap.alphaType());
31-
32- SkBitmap convertedBitmap;
33- if (!convertedBitmap.tryAllocPixels(info)) {
34- return;
35- }
36-
37- bitmap.readPixels(info, convertedBitmap.getPixels(), 0, 0, 0);
38-
39- image = QImage(reinterpret_cast<const uchar *>(convertedBitmap.getPixels()),
40- bitmap.width(),
41- bitmap.height(),
42- QImage::Format_RGBA8888);
43- } else {
44- image = QImage(reinterpret_cast<const uchar *>(bitmap.getPixels()),
45- bitmap.width(),
46- bitmap.height(),
47- QImage::Format_RGBA8888);
48- }
49-
50- write_mime_data_acc_->setImageData(image.copy());
51+ write_mime_data_acc_->setImageData(QImageFromSkBitmap(bitmap));
52 }
53
54 void Clipboard::WriteData(const FormatType& format,
55diff --git a/qt/core/browser/oxide_qt_skutils.cc b/qt/core/browser/oxide_qt_skutils.cc
56index d7bc690..0aecca5 100644
57--- a/qt/core/browser/oxide_qt_skutils.cc
58+++ b/qt/core/browser/oxide_qt_skutils.cc
59@@ -1,5 +1,5 @@
60 // vim:expandtab:shiftwidth=2:tabstop=2:
61-// Copyright (C) 2014 Canonical Ltd.
62+// Copyright (C) 2014-2016 Canonical Ltd.
63
64 // This library is free software; you can redistribute it and/or
65 // modify it under the terms of the GNU Lesser General Public
66@@ -102,12 +102,16 @@ QImage QImageFromSkBitmap(const SkBitmap& bitmap) {
67 QImageFormatFromSkColorType(color, alpha == kPremul_SkAlphaType);
68 if (format != QImage::Format_Invalid) {
69 SkAutoLockPixels lock(bitmap);
70- return QImage(reinterpret_cast<const uchar*>(bitmap.getPixels()),
71- bitmap.width(), bitmap.height(), format);
72+ QImage image(reinterpret_cast<const uchar*>(bitmap.getPixels()),
73+ bitmap.width(), bitmap.height(), format);
74+ image.detach();
75+ return image;
76 }
77
78 if (color == kBGRA_8888_SkColorType) {
79- return QImageFromSkBitmap_BGRA_8888(bitmap);
80+ QImage image = QImageFromSkBitmap_BGRA_8888(bitmap);
81+ image.detach();
82+ return image;
83 }
84
85 LOG(WARNING) <<
86diff --git a/qt/core/browser/oxide_qt_web_context_menu.cc b/qt/core/browser/oxide_qt_web_context_menu.cc
87index 29202e2..acbbeae 100644
88--- a/qt/core/browser/oxide_qt_web_context_menu.cc
89+++ b/qt/core/browser/oxide_qt_web_context_menu.cc
90@@ -1,5 +1,5 @@
91 // vim:expandtab:shiftwidth=2:tabstop=2:
92-// Copyright (C) 2015 Canonical Ltd.
93+// Copyright (C) 2015-2016 Canonical Ltd.
94
95 // This library is free software; you can redistribute it and/or
96 // modify it under the terms of the GNU Lesser General Public
97@@ -169,6 +169,10 @@ int WebContextMenu::mediaFlags() const {
98 return flags;
99 }
100
101+void WebContextMenu::copyImage() const {
102+ CopyImage();
103+}
104+
105 void WebContextMenu::saveLink() const {
106 SaveLink();
107 }
108diff --git a/qt/core/browser/oxide_qt_web_context_menu.h b/qt/core/browser/oxide_qt_web_context_menu.h
109index 8db72d8..257db2b 100644
110--- a/qt/core/browser/oxide_qt_web_context_menu.h
111+++ b/qt/core/browser/oxide_qt_web_context_menu.h
112@@ -1,5 +1,5 @@
113 // vim:expandtab:shiftwidth=2:tabstop=2:
114-// Copyright (C) 2015 Canonical Ltd.
115+// Copyright (C) 2015-2016 Canonical Ltd.
116
117 // This library is free software; you can redistribute it and/or
118 // modify it under the terms of the GNU Lesser General Public
119@@ -60,6 +60,7 @@ class WebContextMenu : public oxide::WebContextMenu,
120 void cancel() override;
121 int editFlags() const override;
122 int mediaFlags() const override;
123+ void copyImage() const override;
124 void saveLink() const override;
125 void saveMedia() const override;
126
127diff --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
128index b5d0131..123297e 100644
129--- a/qt/core/glue/oxide_qt_web_context_menu_proxy_client.h
130+++ b/qt/core/glue/oxide_qt_web_context_menu_proxy_client.h
131@@ -1,5 +1,5 @@
132 // vim:expandtab:shiftwidth=2:tabstop=2:
133-// Copyright (C) 2015 Canonical Ltd.
134+// Copyright (C) 2015-2016 Canonical Ltd.
135
136 // This library is free software; you can redistribute it and/or
137 // modify it under the terms of the GNU Lesser General Public
138@@ -71,6 +71,7 @@ class WebContextMenuProxyClient {
139
140 virtual int mediaFlags() const = 0;
141
142+ virtual void copyImage() const = 0;
143 virtual void saveLink() const = 0;
144 virtual void saveMedia() const = 0;
145 };
146diff --git a/qt/quick/oxide_qquick_web_context_menu.cc b/qt/quick/oxide_qquick_web_context_menu.cc
147index 31b4575..561f421 100644
148--- a/qt/quick/oxide_qquick_web_context_menu.cc
149+++ b/qt/quick/oxide_qquick_web_context_menu.cc
150@@ -1,5 +1,5 @@
151 // vim:expandtab:shiftwidth=2:tabstop=2:
152-// Copyright (C) 2015 Canonical Ltd.
153+// Copyright (C) 2015-2016 Canonical Ltd.
154
155 // This library is free software; you can redistribute it and/or
156 // modify it under the terms of the GNU Lesser General Public
157@@ -71,6 +71,7 @@ class ContextMenuContext : public QObject {
158 OxideQQuickWebView::EditCapabilities editFlags() const;
159 OxideQQuickWebView::MediaStatus mediaFlags() const;
160
161+ Q_INVOKABLE void copyImage() const;
162 Q_INVOKABLE void saveLink() const;
163 Q_INVOKABLE void saveMedia() const;
164
165@@ -214,6 +215,22 @@ OxideQQuickWebView::MediaStatus ContextMenuContext::mediaFlags() const {
166 return static_cast<OxideQQuickWebView::MediaStatus>(client_->mediaFlags());
167 }
168
169+void ContextMenuContext::copyImage() const {
170+ OxideQQuickWebView::MediaType media_type = mediaType();
171+ if (media_type != OxideQQuickWebView::MediaTypeImage &&
172+ media_type != OxideQQuickWebView::MediaTypeCanvas) {
173+ qWarning() << "ContextMenuContext::copyImage(): not an image";
174+ return;
175+ }
176+
177+ if (!hasImageContents()) {
178+ qWarning() << "ContextMenuContext::copyImage(): image has no contents";
179+ return;
180+ }
181+
182+ client_->copyImage();
183+}
184+
185 void ContextMenuContext::saveLink() const {
186 if (linkUrl().isValid()) {
187 client_->saveLink();
188diff --git a/qt/tests/qmltests/api/tst_WebView_contextMenu.qml b/qt/tests/qmltests/api/tst_WebView_contextMenu.qml
189index 6017a1b..f8954b2 100644
190--- a/qt/tests/qmltests/api/tst_WebView_contextMenu.qml
191+++ b/qt/tests/qmltests/api/tst_WebView_contextMenu.qml
192@@ -96,6 +96,14 @@ TestWebView {
193 compare(model.isEditable, data.isEditable);
194 }
195
196+ function test_WebView_contextMenu_copyImage() {
197+ ClipboardTestUtils.clearClipboard();
198+ verify(!ClipboardTestUtils.hasImage());
199+ invokeContextMenu("image");
200+ webView.currentContextMenu.contextModel.copyImage();
201+ verify(TestUtils.waitFor(ClipboardTestUtils.hasImage));
202+ }
203+
204 function test_WebView_contextMenu_saveLink() {
205 invokeContextMenu("hyperlink");
206 var model = webView.currentContextMenu.contextModel;
207diff --git a/qt/tests/qmltests/qml_test_support.cc b/qt/tests/qmltests/qml_test_support.cc
208index 4c464c0..1de24a5 100644
209--- a/qt/tests/qmltests/qml_test_support.cc
210+++ b/qt/tests/qmltests/qml_test_support.cc
211@@ -1,5 +1,5 @@
212 // vim:expandtab:shiftwidth=2:tabstop=2:
213-// Copyright (C) 2013-2015 Canonical Ltd.
214+// Copyright (C) 2013-2016 Canonical Ltd.
215
216 // This library is free software; you can redistribute it and/or
217 // modify it under the terms of the GNU Lesser General Public
218@@ -68,6 +68,10 @@ void ExternalProtocolHandler::setScheme(const QString& scheme) {
219
220 ClipboardTestUtils::ClipboardTestUtils() {}
221
222+bool ClipboardTestUtils::hasImage() const {
223+ return QGuiApplication::clipboard()->mimeData()->hasImage();
224+}
225+
226 void ClipboardTestUtils::copyToClipboard(const QString& mimeType,
227 const QString& data) {
228 QMimeData * mime_data = new QMimeData();
229diff --git a/qt/tests/qmltests/qml_test_support.h b/qt/tests/qmltests/qml_test_support.h
230index 8d400bd..ac0f904 100644
231--- a/qt/tests/qmltests/qml_test_support.h
232+++ b/qt/tests/qmltests/qml_test_support.h
233@@ -58,6 +58,8 @@ class ClipboardTestUtils : public QObject {
234 public:
235 ClipboardTestUtils();
236
237+ Q_INVOKABLE bool hasImage() const;
238+
239 Q_INVOKABLE void copyToClipboard(const QString& mimeType,
240 const QString& data);
241
242diff --git a/shared/browser/oxide_web_context_menu.cc b/shared/browser/oxide_web_context_menu.cc
243index 718b7f9..6efa9f2 100644
244--- a/shared/browser/oxide_web_context_menu.cc
245+++ b/shared/browser/oxide_web_context_menu.cc
246@@ -1,5 +1,5 @@
247 // vim:expandtab:shiftwidth=2:tabstop=2:
248-// Copyright (C) 2015 Canonical Ltd.
249+// Copyright (C) 2015-2016 Canonical Ltd.
250
251 // This library is free software; you can redistribute it and/or
252 // modify it under the terms of the GNU Lesser General Public
253@@ -79,6 +79,13 @@ void WebContextMenu::Close() {
254 content::BrowserThread::UI, FROM_HERE, this);
255 }
256
257+void WebContextMenu::CopyImage() const {
258+ if ((params_.media_type == blink::WebContextMenuData::MediaTypeCanvas) ||
259+ (params_.media_type == blink::WebContextMenuData::MediaTypeImage)) {
260+ render_frame_host_->GetRenderViewHost()->CopyImageAt(params_.x, params_.y);
261+ }
262+}
263+
264 void WebContextMenu::SaveLink() const {
265 content::BrowserContext* context = web_contents()->GetBrowserContext();
266 content::DownloadManager* dlm =
267diff --git a/shared/browser/oxide_web_context_menu.h b/shared/browser/oxide_web_context_menu.h
268index 24f9277..8ca6d35 100644
269--- a/shared/browser/oxide_web_context_menu.h
270+++ b/shared/browser/oxide_web_context_menu.h
271@@ -1,5 +1,5 @@
272 // vim:expandtab:shiftwidth=2:tabstop=2:
273-// Copyright (C) 2015 Canonical Ltd.
274+// Copyright (C) 2015-2016 Canonical Ltd.
275
276 // This library is free software; you can redistribute it and/or
277 // modify it under the terms of the GNU Lesser General Public
278@@ -56,6 +56,7 @@ class OXIDE_SHARED_EXPORT WebContextMenu : public content::WebContentsObserver {
279
280 content::ContextMenuParams params_;
281
282+ void CopyImage() const;
283 void SaveLink() const;
284 void SaveMedia() const;
285

Subscribers

People subscribed via source and target branches