webbrowser-app

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

addressBarFullWidth
Merge into trunk

Proposed by Riccardo Padovani on 2014-10-21

Status:

Needs review

Proposed branch:

lp://staging/~rpadovani/webbrowser-app/addressBarFullWidth

Merge into:

lp://staging/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://staging/~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.staging.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://staging/~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://staging/~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://staging/~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://staging/~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://staging/~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://staging/~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://staging/~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://staging/~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://staging/~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