webbrowser-app

Merge lp:~rpadovani/webbrowser-app/addressBarFullWidth into lp:webbrowser-app

addressBarFullWidth
Merge into trunk

Proposed by Riccardo Padovani on 2014-10-21

Status:

Needs review

Proposed branch:

lp:~rpadovani/webbrowser-app/addressBarFullWidth

Merge into:

lp:webbrowser-app

Diff against target:

162 lines (+33/-5)

5 files modified

debian/control (+2/-0)
src/app/webbrowser/AddressBar.qml (+10/-2)
src/app/webbrowser/Chrome.qml (+9/-3)
tests/autopilot/webbrowser_app/tests/test_addressbar_states.py (+10/-0)
tests/autopilot/webbrowser_app/tests/test_suggestions.py (+2/-0)

To merge this branch:

bzr merge lp:~rpadovani/webbrowser-app/addressBarFullWidth

High

Invalid

Link a bug report

Reviewer	Review Type	Date Requested	Status
PS Jenkins bot	continuous-integration		Needs Fixing on 2015-06-12
Olivier Tilloy		2014-10-21	Needs Fixing on 2015-01-28
Review via email: mp+239039@code.launchpad.net

Commit message

Set addressbar at full width when is focused, as per design

Description of the change

Set addressbar at full width when is focused, as per design

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2014-10-21:

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

review: Needs Fixing (continuous-integration)

Revision history for this message

Olivier Tilloy (osomon) wrote on 2014-11-12:

The change looks good, but it breaks a large number of autopilot tests in at least two different ways:

- Some tests focus the address bar and then try to open the drawer, but the button is hidden. Some of these tests can simply be removed, and some will need to be rewritten to account for the new behaviour.

- A number of tests focus the address bar and then try to click the clear button, but since the width of the address bar is animated, in the time it takes for the cursor to move to where the clear button was, it has moved away, and the click happens outside the button. This is mostly an issue on desktop, but could potentially affect touch devices as well. We need a way for the autopilot tests to wait until the address bar has finished expanding before clicking the clear button.

review: Needs Fixing

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2014-12-19:

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

review: Needs Fixing (continuous-integration)

Revision history for this message

Riccardo Padovani (rpadovani) wrote on 2015-01-22:

Sorry, but I needed 2 months to understand autopilot :P

Jokes aside, I deleted test_cannot_bookmark_empty_page because when you open a new tab the address bar is focused and users cannot unfocus it, so he can't click anymore the bookmark menu.

I also deleted test_suggestions_hidden_while_drawer_open because now user can't open the drawer while the address bar is focused

I fixed test_selecting_tab_focuses_webview adding a click to the middle of the page to unfocus the address bar

I fixed also test_special_characters adding a delay before the clear button is clicked

test_looses_focus_when_reloading it's a special case: I fixed it (so now it clicks on the button), but fails anywhere: this evidence a real problem. Indeed, on top of some pixels at the center of the reload icon, (where the autopilot presses), there is a MouseArea that activates the tooltip for copy/paste. I spend some time on it, but I didn't understand the root cause. I'll do further investigation in next days. Meanwhile, if you want, try to take a look.

I think is a toolkit bug - for sure it's not an autopilot issue, because I'm able to reproduce it, so before merging that branch I have to fix the problem

lp:~rpadovani/webbrowser-app/addressBarFullWidth updated on 2015-01-22

763. By Riccardo Padovani on 2015-01-22: Merged from trunk
764. By Riccardo Padovani on 2015-01-22: fixing autopilot

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2015-01-23:

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

review: Needs Fixing (continuous-integration)

lp:~rpadovani/webbrowser-app/addressBarFullWidth updated on 2015-01-24

765. By Riccardo Padovani on 2015-01-24: Hide left action in the address bar when editing
766. By Riccardo Padovani on 2015-01-24: Fixed webbrowser_app.tests.test_addressbar_states.TestAddressBarStates.test_looses_focus_when_reloading

Revision history for this message

Riccardo Padovani (rpadovani) wrote on 2015-01-24:

Actually, upstream fixed bug #1384278, so per design we can hide the left action in the edit mode.

So I changed the code to hide left action in edit mode and to change keyboard status.

I also changed webbrowser_app.tests.test_addressbar_states.TestAddressBarStates.test_looses_focus_when_reloading to use "Enter" on keyboard instead of clicking the reload button.

The branch now should be ready to be merged

lp:~rpadovani/webbrowser-app/addressBarFullWidth updated on 2015-01-24

767. By Riccardo Padovani on 2015-01-24: Merged from trunk

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2015-01-24:

FAILED: Continuous integration, rev:767
http://jenkins.qa.ubuntu.com/job/webbrowser-app-ci/1387/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-vivid-touch/969/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-vivid/442/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/webbrowser-app-vivid-amd64-ci/145/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/webbrowser-app-vivid-armhf-ci/145/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/webbrowser-app-vivid-i386-ci/145/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-vivid-armhf/967/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-vivid-amd64/534/console

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

review: Needs Fixing (continuous-integration)

Revision history for this message

Olivier Tilloy (osomon) wrote on 2015-01-24:

There are several build failures that need fixing:

tests/autopilot/webbrowser_app/tests/test_addressbar_states.py:51:44: F821 undefined name 'Eventually'
tests/autopilot/webbrowser_app/tests/test_addressbar_states.py:51:55: F821 undefined name 'Equals'

and

src/app/webbrowser/AddressBar.qml:20:1: module "Ubuntu.Keyboard" is not installed
import Ubuntu.Keyboard 0.1

review: Needs Fixing

Revision history for this message

Olivier Tilloy (osomon) wrote on 2015-01-26:

28 + visible: addressbar.state != "editing"
76 + visible: addressbar.state != 'editing'
85 + visible: enabled && addressbar.state != 'editing'
110 + visible: addressbar.state != 'editing'

The "editing" state of AddressBar was recently removed. Instead, use addressbar.activeFocus.

review: Needs Fixing

lp:~rpadovani/webbrowser-app/addressBarFullWidth updated on 2015-01-26

768. By Riccardo Padovani on 2015-01-26: Fixed autopilot import
769. By Riccardo Padovani on 2015-01-26: Added qtdeclarative5-ubuntu-keyboard-extensions0.1 to debian/control

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2015-01-26:

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

review: Needs Fixing (continuous-integration)

lp:~rpadovani/webbrowser-app/addressBarFullWidth updated on 2015-01-26

770. By Riccardo Padovani on 2015-01-26: merged from upstream
771. By Riccardo Padovani on 2015-01-26: Sobstituted addressbar.state != 'editing' with addressbar.activeFocus

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2015-01-26:

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

review: Needs Fixing (continuous-integration)

Revision history for this message

Francis Ginther (fginther) wrote on 2015-01-26:

The jenkins node for the i386 build failed, I've restarted a new ci run.

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2015-01-27:

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

review: Needs Fixing (continuous-integration)

Revision history for this message

Olivier Tilloy (osomon) wrote on 2015-01-28:

There is still one comparison of addressbar.state with "editing" in src/app/webbrowser/AddressBar.qml.

review: Needs Fixing

Revision history for this message

Olivier Tilloy (osomon) wrote on 2015-01-28:

The change to test_looses_focus_when_reloading makes the test meaningless (or at the very least the name needs to be updated too): the test was about clicking on the reload button, not validating the current URL, which takes a different code path.

review: Needs Fixing

Revision history for this message

Olivier Tilloy (osomon) wrote on 2015-01-28:

> So I changed the code to hide left action in edit mode and to change keyboard
> status.

I’m not sure it should always be hidden. At the very least, not on desktop where there is no OSK by default.

Revision history for this message

Riccardo Padovani (rpadovani) wrote on 2015-02-01:

Okay, so:
- reload icon is hiddend only when virtual keyboard is visibile
- test runs only on desktop

as we discussed Friday

The test fails, but I don't think it's related to this branch, because it fails also on trunk: there is a little square in the middle of the reload icon where, for I don't know which reason, the click opens the copy/paste dialog

lp:~rpadovani/webbrowser-app/addressBarFullWidth updated on 2015-02-02

772. By Riccardo Padovani on 2015-02-01: Implemented case for physical keyboard
773. By Riccardo Padovani on 2015-02-01: fixed test_looses_focus_when_reloading
774. By Riccardo Padovani on 2015-02-01: merged from upstream

Revision history for this message

Riccardo Padovani (rpadovani) wrote on 2015-02-02:

Argh, forgot to push, sorry :S

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2015-02-03:

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

review: Needs Fixing (continuous-integration)

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2015-02-03:

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

review: Needs Fixing (continuous-integration)

lp:~rpadovani/webbrowser-app/addressBarFullWidth updated on 2015-02-25

775. By Riccardo Padovani on 2015-02-25: Merged from upstream and fixed conflicts

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2015-02-25:

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

review: Needs Fixing (continuous-integration)

lp:~rpadovani/webbrowser-app/addressBarFullWidth updated on 2015-03-02

776. By Riccardo Padovani on 2015-03-02: Merged from trunk
777. By Riccardo Padovani on 2015-03-02: removed qtdeclarative5-private-dev

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2015-03-02:

FAILED: Continuous integration, rev:775
http://jenkins.qa.ubuntu.com/job/webbrowser-app-ci/1503/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-vivid-touch/1595/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/webbrowser-app-vivid-amd64-ci/261/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/webbrowser-app-vivid-armhf-ci/261/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/webbrowser-app-vivid-i386-ci/261/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-vivid-armhf/1593/console

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

review: Needs Fixing (continuous-integration)

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2015-03-02:

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

review: Needs Fixing (continuous-integration)

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2015-03-02:

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

review: Needs Fixing (continuous-integration)

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2015-03-02:

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

review: Needs Fixing (continuous-integration)

lp:~rpadovani/webbrowser-app/addressBarFullWidth updated on 2015-06-12

778. By Riccardo Padovani on 2015-06-12: Merge from upstream and fix conflicts

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2015-06-12:

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

review: Needs Fixing (continuous-integration)

Unmerged revisions

778. By Riccardo Padovani on 2015-06-12: Merge from upstream and fix conflicts
777. By Riccardo Padovani on 2015-03-02: removed qtdeclarative5-private-dev
776. By Riccardo Padovani on 2015-03-02: Merged from trunk
775. By Riccardo Padovani on 2015-02-25: Merged from upstream and fixed conflicts
774. By Riccardo Padovani on 2015-02-01: merged from upstream
773. By Riccardo Padovani on 2015-02-01: fixed test_looses_focus_when_reloading
772. By Riccardo Padovani on 2015-02-01: Implemented case for physical keyboard
771. By Riccardo Padovani on 2015-01-26: Sobstituted addressbar.state != 'editing' with addressbar.activeFocus
770. By Riccardo Padovani on 2015-01-26: merged from upstream
769. By Riccardo Padovani on 2015-01-26: Added qtdeclarative5-ubuntu-keyboard-extensions0.1 to debian/control

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk

Subscribers

People subscribed via source and target branches

to all changes:

Riccardo Padovani

Ubuntu Phablet Team

to status/vote changes:

Rachel Liu

1	=== modified file 'debian/control'
2	--- debian/control 2015-05-27 05:31:12 +0000
3	+++ debian/control 2015-06-12 09:40:41 +0000
4	@@ -21,6 +21,7 @@
5	qtdeclarative5-dev,
6	qtdeclarative5-private-dev,
7	qtdeclarative5-ubuntu-ui-toolkit-plugin,
8	+ qtdeclarative5-ubuntu-keyboard-extensions0.1,
9	xvfb,
10	Standards-Version: 3.9.5
11	Homepage: https://launchpad.net/webbrowser-app
12	@@ -41,6 +42,7 @@
13	qml-module-qtquick2 (>= 5.4) \| qtdeclarative5-qtquick2-plugin (>= 5.4),
14	qml-module-qtquick-dialogs \| qtdeclarative5-dialogs-plugin,
15	qml-module-qtquick-window2 \| qtdeclarative5-window-plugin,
16	+ qtdeclarative5-ubuntu-keyboard-extensions0.1,
17	qtdeclarative5-ubuntu-ui-toolkit-plugin,
18	qtdeclarative5-ubuntu-web-plugin (= ${binary:Version}),
19	qtdeclarative5-unity-action-plugin,
20
21	=== modified file 'src/app/webbrowser/AddressBar.qml'
22	--- src/app/webbrowser/AddressBar.qml 2015-05-26 21:42:54 +0000
23	+++ src/app/webbrowser/AddressBar.qml 2015-06-12 09:40:41 +0000
24	@@ -17,6 +17,7 @@
25	*/
26
27	import QtQuick 2.0
28	+import Ubuntu.Keyboard 0.1
29	import Ubuntu.Components 1.1
30	import Ubuntu.Components.Popups 1.0
31	import com.canonical.Oxide 1.0 as Oxide
32	@@ -57,9 +58,11 @@
33	primaryItem: Item {
34	id: icons
35
36	- width: iconsRow.width + units.gu(1)
37	+ width: visible ? iconsRow.width + units.gu(1) : 0
38	height: units.gu(2)
39
40	+ visible: !(Qt.inputMethod.visible && addressbar.activeFocus)
41	+
42	Row {
43	id: iconsRow
44
45	@@ -84,7 +87,8 @@
46	height: parent.height
47	width: height
48
49	- visible: addressbar.activeFocus \|\| addressbar.loading \|\| !addressbar.text
50	+ visible: (addressbar.activeFocus && !Qt.inputMethod.visible) \|\|
51	+ addressbar.loading \|\| !addressbar.text
52
53	enabled: addressbar.text
54	opacity: enabled ? 1.0 : 0.3
55	@@ -97,6 +101,10 @@
56	reload ? "reload" :
57	looksLikeAUrl ? "stock_website" : "search"
58
59	+ InputMethod.extensions: {
60	+ "enterKeyIconSource": action.reload ? "reload" : "keyboard-enter"
61	+ }
62	+
63	MouseArea {
64	objectName: "actionButton"
65
66
67	=== modified file 'src/app/webbrowser/Chrome.qml'
68	--- src/app/webbrowser/Chrome.qml 2015-05-26 21:42:54 +0000
69	+++ src/app/webbrowser/Chrome.qml 2015-06-12 09:40:41 +0000
70	@@ -50,7 +50,9 @@
71	iconColor: internal.iconColor
72
73	height: chrome.height
74	- width: height * 0.8
75	+ width: visible ? height * 0.8 : 0
76	+
77	+ visible: !addressbar.activeFocus
78
79	anchors {
80	left: parent.left
81	@@ -70,7 +72,7 @@
82	iconColor: internal.iconColor
83
84	height: chrome.height
85	- visible: enabled
86	+ visible: enabled && !addressbar.activeFocus
87	width: visible ? height * 0.8 : 0
88
89	anchors {
90	@@ -103,10 +105,12 @@
91	chrome.webview.forceActiveFocus()
92	chrome.webview.url = requestedUrl
93	}
94	+
95	onRequestReload: {
96	chrome.webview.forceActiveFocus()
97	chrome.webview.reload()
98	}
99	+
100	onRequestStop: chrome.webview.stop()
101
102	Connections {
103	@@ -136,7 +140,9 @@
104	iconColor: internal.iconColor
105
106	height: chrome.height
107	- width: height * 0.8
108	+ width: visible ? height * 0.8: 0
109	+
110	+ visible: !addressbar.activeFocus
111
112	anchors {
113	right: parent.right
114
115	=== modified file 'tests/autopilot/webbrowser_app/tests/test_addressbar_states.py'
116	--- tests/autopilot/webbrowser_app/tests/test_addressbar_states.py 2015-02-17 20:52:30 +0000
117	+++ tests/autopilot/webbrowser_app/tests/test_addressbar_states.py 2015-06-12 09:40:41 +0000
118	@@ -14,8 +14,14 @@
119	# You should have received a copy of the GNU General Public License
120	# along with this program. If not, see <http://www.gnu.org/licenses/>.
121
122	+from testtools.matchers import Equals
123	+from autopilot.matchers import Eventually
124	+from autopilot.platform import model
125	+
126	from webbrowser_app.tests import StartOpenRemotePageTestCaseBase
127
128	+import unittest
129	+
130
131	class TestAddressBarStates(StartOpenRemotePageTestCaseBase):
132
133	@@ -42,6 +48,7 @@
134	self.main_window.go_to_url(url)
135	address_bar.activeFocus.wait_for(False)
136
137	+ @unittest.skipIf(model() != "Desktop", "on desktop only")
138	def test_looses_focus_when_reloading(self):
139	address_bar = self.main_window.address_bar
140	self.pointing_device.click_object(address_bar)
141	@@ -51,5 +58,8 @@
142	# button.
143	address_bar.clear()
144	address_bar.write(self.url)
145	+ chrome = self.main_window.chrome
146	+ drawer_button = chrome.get_drawer_button()
147	+ self.assertThat(drawer_button.opacity, Eventually(Equals(1)))
148	address_bar.click_action_button()
149	address_bar.activeFocus.wait_for(False)
150
151	=== modified file 'tests/autopilot/webbrowser_app/tests/test_suggestions.py'
152	--- tests/autopilot/webbrowser_app/tests/test_suggestions.py 2015-05-20 05:45:29 +0000
153	+++ tests/autopilot/webbrowser_app/tests/test_suggestions.py 2015-06-12 09:40:41 +0000
154	@@ -285,6 +285,8 @@
155	self.assertThat(webview.url, Eventually(Contains(url)))
156
157	def test_special_characters(self):
158	+ self.address_bar.focus()
159	+ self.assert_suggestions_eventually_shown()
160	self.address_bar.clear()
161	self.address_bar.write('(phil')
162	self.assert_suggestions_eventually_shown()