Merge lp:~osomon/webbrowser-app/cannot-bookmark-empty-tab into lp:webbrowser-app/staging

Proposed by Olivier Tilloy on 2017-03-28
Status: Merged
Approved by: Andrew Hayzen on 2017-03-28
Approved revision: 1640
Merged at revision: 1640
Proposed branch: lp:~osomon/webbrowser-app/cannot-bookmark-empty-tab
Merge into: lp:webbrowser-app/staging
Diff against target: 54 lines (+18/-2)
2 files modified
src/app/webbrowser/Browser.qml (+1/-1)
tests/autopilot/webbrowser_app/tests/test_keyboard.py (+17/-1)
To merge this branch: bzr merge lp:~osomon/webbrowser-app/cannot-bookmark-empty-tab
Reviewer Review Type Date Requested Status
Andrew Hayzen (community) 2017-03-28 Approve on 2017-03-28
Review via email: mp+321166@code.launchpad.net

Commit message

Do not allow bookmarking an empty tab.

To post a comment you must log in.
Andrew Hayzen (ahayzen) wrote :

LGTM, and the test pass :-)

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'src/app/webbrowser/Browser.qml'
2--- src/app/webbrowser/Browser.qml 2017-03-03 14:49:25 +0000
3+++ src/app/webbrowser/Browser.qml 2017-03-28 11:21:53 +0000
4@@ -1348,7 +1348,7 @@
5 // Ctrl+D: Toggle bookmarked state on current Tab
6 Shortcut {
7 sequence: "Ctrl+D"
8- enabled: contentsContainer.visible
9+ enabled: contentsContainer.visible && !newTabViewLoader.active
10 onActivated: {
11 if (internal.currentBookmarkOptionsDialog) {
12 internal.currentBookmarkOptionsDialog.hide()
13
14=== modified file 'tests/autopilot/webbrowser_app/tests/test_keyboard.py'
15--- tests/autopilot/webbrowser_app/tests/test_keyboard.py 2016-09-28 08:23:23 +0000
16+++ tests/autopilot/webbrowser_app/tests/test_keyboard.py 2017-03-28 11:21:53 +0000
17@@ -1,6 +1,6 @@
18 # -*- Mode: Python; coding: utf-8; indent-tabs-mode: nil; tab-width: 4 -*-
19 #
20-# Copyright 2015-2016 Canonical
21+# Copyright 2015-2017 Canonical
22 #
23 # This program is free software: you can redistribute it and/or modify it
24 # under the terms of the GNU General Public License version 3, as published
25@@ -20,6 +20,7 @@
26 import testtools
27
28 from testtools.matchers import Equals, Mismatch, NotEquals, GreaterThan
29+from autopilot import exceptions
30 from autopilot.matchers import Eventually
31 from autopilot.platform import model
32
33@@ -261,6 +262,21 @@
34 self.main_window.press_key('Ctrl+d')
35 self.assertThat(chrome.bookmarked, Eventually(Equals(False)))
36
37+ def test_cannot_bookmark_empty_tab(self):
38+ self.main_window.press_key('Ctrl+t')
39+ webview = self.main_window.get_current_webview()
40+ self.assertThat(webview.url, Equals(""))
41+ new_tab_view = self.main_window.get_new_tab_view()
42+ self.assertThat(new_tab_view.visible, Eventually(Equals(True)))
43+ self.main_window.press_key('Ctrl+d')
44+ time.sleep(2)
45+ try:
46+ self.main_window.get_bookmark_options()
47+ except exceptions.StateNotFoundError:
48+ pass
49+ else:
50+ self.fail("Bookmarking an empty tab should not be allowed")
51+
52 def test_history_navigation_with_alt_arrows(self):
53 previous = self.main_window.get_current_webview().url
54 url = self.base_url + "/test2"

Subscribers

People subscribed via source and target branches