Merge lp:~fboucault/webbrowser-app/workaround_new_tab_never_unloaded into lp:webbrowser-app/staging

[Florian Boucault]

Fix issue with new tab page sometimes never being unloaded

Fix issue with new tab page sometimes never being unloaded

[Olivier Tilloy (osomon)]

This breaks webbrowser_app.tests.test_new_tab_view.TestNewTabViewLifetime.test_new_tab_view_is_shared_between_tabs

review: Needs Fixing

[lp:~fboucault/webbrowser-app/workaround_new_tab_never_unloaded]

1607. By Florian Boucault on 2017-02-15

Merged staging

1608. By Florian Boucault on 2017-02-15

Fix AP test webbrowser_app.tests.test_new_tab_view.TestNewTabViewLifetime.test_new_tab_view_is_shared_between_tabs

1609. By Florian Boucault on 2017-02-15

Clarified and centralised showing empty tab logic.

1610. By Florian Boucault on 2017-02-15

BrowserTab: made empty property readonly

[Olivier Tilloy (osomon)]

LGTM, thanks!

review: Approve

=== modified file 'src/app/webbrowser/Browser.qml'
--- src/app/webbrowser/Browser.qml	2017-02-06 22:17:36 +0000
+++ src/app/webbrowser/Browser.qml	2017-02-15 14:56:57 +0000
@@ -264,14 +264,14 @@
             }
             clip: true  // prevents component from overlapping bottom edge etc
 
-            // Avoid loading the new tab view if the webview is about to load
-            // content. Since WebView.restoreState is not a notifyable property,
-            // this can’t be achieved with a simple property binding.
-            Connections {
-                target: currentWebview
-                onUrlChanged: {
-                    newTabViewLoader.active = false
-                }
+            // Avoid loading the new tab view if the webview has or will have content
+            Connections {
+                target: tabsModel.currentTab
+                onEmptyChanged: newTabViewLoader.setActive(tabsModel.currentTab.empty)
+            }
+            Connections {
+                target: tabsModel
+                onCurrentTabChanged: newTabViewLoader.setActive(tabsModel.currentTab && tabsModel.currentTab.empty)
             }
             active: false
             focus: active
@@ -279,16 +279,26 @@
 
             Connections {
                 target: browser
-                onCurrentWebviewChanged: {
-                    if (currentWebview) {
-                        var tab = tabsModel.currentTab
-                        newTabViewLoader.active = !tab.url.toString() && !tab.restoreState
-                    }
-                }
                 onWideChanged: newTabViewLoader.selectTabView()
             }
             Component.onCompleted: newTabViewLoader.selectTabView()
 
+            // FIXME: this is a workaround for bug #1659435 caused by QTBUG-54657.
+            // New tab view was sometimes overlaid on top of other tabs because
+            // it was not unloaded even though active was set to false.
+            // Ref.: https://bugs.launchpad.net/ubuntu/+source/webbrowser-app/+bug/1659435
+            //       https://bugreports.qt.io/browse/QTBUG-54657
+            function setActive(active) {
+                if (active) {
+                    if (newTabViewLoader.source == "") {
+                        selectTabView();
+                    }
+                } else {
+                    newTabViewLoader.setSource("", {});
+                }
+                newTabViewLoader.active = active;
+            }
+
             function selectTabView() {
                 var source = browser.incognito ? "NewPrivateTabView.qml" :
                                                  (browser.wide ? "NewTabViewWide.qml" :
@@ -1114,7 +1124,7 @@
                 if (recentView.visible) {
                     recentView.focus = true
                 } else if (tab) {
-                    if (!tab.url.toString() && !tab.initialUrl.toString()) {
+                    if (tab.empty) {
                         maybeFocusAddressBar()
                     } else {
                         tabContainer.forceActiveFocus()
@@ -1130,8 +1140,9 @@
         }
 
         function resetFocus() {
-            if (browser.currentWebview) {
-                if (!browser.currentWebview.url.toString()) {
+            var currentTab = tabsModel.currentTab;
+            if (currentTab) {
+                if (currentTab.empty) {
                     internal.maybeFocusAddressBar()
                 } else {
                     contentsContainer.focus = true;

=== modified file 'src/app/webbrowser/BrowserTab.qml'
--- src/app/webbrowser/BrowserTab.qml	2016-11-08 14:19:52 +0000
+++ src/app/webbrowser/BrowserTab.qml	2017-02-15 14:56:57 +0000
@@ -42,6 +42,7 @@
     property bool current: false
     readonly property real lastCurrent: internal.lastCurrent
     property bool incognito
+    readonly property bool empty: !url.toString() && !initialUrl.toString() && !restoreState && !request
     visible: false
 
     // Used as a workaround for https://launchpad.net/bugs/1502675 :