Merge lp:~osomon/oxide/tolerant-GURL2QUrl into lp:~oxide-developers/oxide/oxide.trunk

Proposed by Olivier Tilloy on 2015-04-24
Status: Rejected
Rejected by: Olivier Tilloy on 2016-05-10
Proposed branch: lp:~osomon/oxide/tolerant-GURL2QUrl
Merge into: lp:~oxide-developers/oxide/oxide.trunk
Diff against target: 44 lines (+25/-1)
1 file modified
qt/core/browser/oxide_qt_browser_platform_integration.cc (+25/-1)
To merge this branch: bzr merge lp:~osomon/oxide/tolerant-GURL2QUrl
Reviewer Review Type Date Requested Status
Olivier Tilloy Disapprove on 2016-05-10
Chris Coulson 2015-04-24 Pending
Review via email: mp+257331@code.launchpad.net

Commit message

Be more tolerant when converting a GURL to a QUrl when calling QDesktopServices::openUrl().

Description of the change

Note: I’m not sure whether this tolerant GURL2QUrl function should be used everywhere we translate a GURL into a QUrl (quite a few places in the codebase), or if it’s ok to keep it confined there.

To post a comment you must log in.
lp:~osomon/oxide/tolerant-GURL2QUrl updated on 2015-04-24
1057. By Olivier Tilloy on 2015-04-24

Add a debug check.

Olivier Tilloy (osomon) wrote :

Not needed any longer as bug #1447617 is invalid now.

review: Disapprove

Unmerged revisions

1057. By Olivier Tilloy on 2015-04-24

Add a debug check.

1056. By Olivier Tilloy on 2015-04-24

Be more tolerant when converting a GURL to a QUrl when calling QDesktopServices::openUrl().

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'qt/core/browser/oxide_qt_browser_platform_integration.cc'
2--- qt/core/browser/oxide_qt_browser_platform_integration.cc 2015-03-24 17:06:40 +0000
3+++ qt/core/browser/oxide_qt_browser_platform_integration.cc 2015-04-24 09:02:06 +0000
4@@ -27,6 +27,7 @@
5 #include <QtGui/qpa/qplatformnativeinterface.h>
6
7 #include "base/lazy_instance.h"
8+#include "base/logging.h"
9 #include "base/message_loop/message_loop_proxy.h"
10 #include "content/public/browser/browser_thread.h"
11 #include "url/gurl.h"
12@@ -46,8 +47,31 @@
13 namespace {
14 base::LazyInstance<QPointer<QThread> > g_io_thread;
15
16+QUrl GURL2QUrl(const GURL& url) {
17+ if (!url.is_valid()) {
18+ return QUrl();
19+ }
20+ QUrl parsed = QUrl(QString::fromStdString(url.spec()));
21+ if (parsed.isValid()) {
22+ return parsed;
23+ }
24+ // Special case where GURL’s parser is more tolerant than QUrl’s
25+ // (see https://launchpad.net/bugs/1447617).
26+ DCHECK(!url.has_host());
27+ QUrl tolerant;
28+ tolerant.setScheme(QString::fromStdString(url.scheme()));
29+ tolerant.setPath(QString::fromStdString(url.path()));
30+ if (url.has_query()) {
31+ tolerant.setQuery(QString::fromStdString(url.query()));
32+ }
33+ if (url.has_ref()) {
34+ tolerant.setFragment(QString::fromStdString(url.ref()));
35+ }
36+ return tolerant;
37+}
38+
39 void LaunchURLExternallyOnUIThread(const GURL& url) {
40- QDesktopServices::openUrl(QUrl(QString::fromStdString(url.spec())));
41+ QDesktopServices::openUrl(GURL2QUrl(url));
42 }
43
44 }

Subscribers

People subscribed via source and target branches