Merge lp:~osomon/webbrowser-app/hyperlinks-blank-target into lp:webbrowser-app

[Olivier Tilloy]

Use javascript event delegation to work around the lack of support for handling hyperlinks with a target attribute set to '_blank' in QtWebKit.

Note to the reviewer: bug #1129281 was originally reported against the twitter web app. Unfortunately, this MR doesn’t fix the original issue because the twitter web app does additional event handling that conflicts with my solution (see https://bugs.launchpad.net/manhattan/+bug/1129281/comments/7 for details).

However this patch does fix the handling of "normal" hyperlinks with target="_blank" (hopefully the majority of them out there), so I think it’s still valuable on its own. It can be tested against http://news.google.com (all links have target="_blank" on this page).

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:89
http://jenkins.qa.ubuntu.com/job/webbrowser-app-ci/7/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/webbrowser-app-quantal-armhf-ci/7
        deb: http://jenkins.qa.ubuntu.com/job/webbrowser-app-quantal-armhf-ci/7/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/webbrowser-app-quantal-i386-ci/7
    SUCCESS: http://jenkins.qa.ubuntu.com/job/webbrowser-app-raring-armhf-ci/4
        deb: http://jenkins.qa.ubuntu.com/job/webbrowser-app-raring-armhf-ci/4/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/webbrowser-app-raring-i386-ci/4
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-runner/305

Click here to trigger a rebuild:
http://s-jenkins:8080/job/webbrowser-app-ci/7/rebuild

review: Approve (continuous-integration)

[Ugo Riboni (uriboni)]

A different approach would be to wait for the document to finish loading, then do document.getElementsByTagName("a") and add your click listener on those of them that have target = "_blank"

The advantage of that is that you don't have to walk up the tree for each click anywhere on the document (which probably happens often). The tradeoff is bviously of some more initial setup at page load, but it shouldn't be too bad.

[Olivier Tilloy (osomon)]

As discussed on IRC, the approach suggested by Ugo would probably consume more memory (for each onclick handler), as opposed to more CPU consumption for the patch I’m proposing. We’d need numbers to back this hypothesis though.

Regardless, this is a temporary solution, the proper way of fixing this issue is to patch QtWebKit to implement decidePolicyForNewWindowAction(…).

This patch improves user experience by enabling something that doesn’t work without it, so let’s get it in as is, and work on a proper fix asap.

=== modified file 'src/Ubuntu/Browser/Browser.qml'
--- src/Ubuntu/Browser/Browser.qml	2013-03-20 18:28:04 +0000
+++ src/Ubuntu/Browser/Browser.qml	2013-03-25 12:57:10 +0000
@@ -130,7 +130,8 @@
         experimental.devicePixelRatio: browser.qtwebkitdpr
         experimental.preferences.developerExtrasEnabled: browser.developerExtrasEnabled
         experimental.preferences.navigatorQtObjectEnabled: true
-        experimental.userScripts: [Qt.resolvedUrl("selection.js")]
+        experimental.userScripts: [Qt.resolvedUrl("hyperlinks.js"),
+                                   Qt.resolvedUrl("selection.js")]
         experimental.onMessageReceived: {
             var data = null
             try {

=== added file 'src/Ubuntu/Browser/hyperlinks.js'
--- src/Ubuntu/Browser/hyperlinks.js	1970-01-01 00:00:00 +0000
+++ src/Ubuntu/Browser/hyperlinks.js	2013-03-25 12:57:10 +0000
@@ -0,0 +1,39 @@
+/*
+ * Copyright 2013 Canonical Ltd.
+ *
+ * This file is part of ubuntu-browser.
+ *
+ * ubuntu-browser is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; version 3.
+ *
+ * ubuntu-browser is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+var doc = document.documentElement;
+
+doc.addEventListener('click', function(event) {
+    var node = event.target;
+    while (node) {
+        if (node.nodeName.toLowerCase() == 'a') {
+            // Using event delegation to work around the lack of support for
+            // handling hyperlinks with a target attribute set to '_blank' in
+            // QtWebKit. See related upstream bug reports:
+            //    https://bugs.webkit.org/show_bug.cgi?id=76416
+            //    https://bugs.webkit.org/show_bug.cgi?id=91779
+            if (node.hasAttribute('target')) {
+                if (node.getAttribute('target').toLowerCase() == '_blank') {
+                    window.location = node.getAttribute('href');
+                }
+            }
+            break;
+        }
+        node = node.parentNode;
+    }
+});