Merge into trunk : new-tab-wide-format : Code : webbrowser-app

Reviewer	Review Type	Date Requested	Status
Olivier Tilloy		2015-07-30	Approve on 2015-08-12
PS Jenkins bot	continuous-integration		Needs Fixing on 2015-08-12
Review via email: mp+266382@code.qastaging.launchpad.net

lp://qastaging/~uriboni/webbrowser-app/new-tab-wide-format updated on 2015-07-30

1075. By Ugo Riboni on 2015-07-30: Merge changes from trunk
1076. By Ugo Riboni on 2015-07-30: Remove useless import and fix flake8 errors
1077. By Ugo Riboni on 2015-07-30: Revert changes to UrlDelegate.qml as we are now using a new component instead

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2015-07-30:

#

FAILED: Continuous integration, rev:1075
http://jenkins.qa.ubuntu.com/job/webbrowser-app-ci/2010/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-vivid-touch/3514/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/webbrowser-app-vivid-amd64-ci/764/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/webbrowser-app-vivid-armhf-ci/764/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/webbrowser-app-vivid-i386-ci/764/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-vivid-armhf/3511/console

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

review: Needs Fixing (continuous-integration)

lp://qastaging/~uriboni/webbrowser-app/new-tab-wide-format updated on 2015-07-30

1078. By Ugo Riboni on 2015-07-30: Fix copyright header
1079. By Ugo Riboni on 2015-07-30: Revert changes to UrlsList as we are not using it anymore

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2015-07-30:

#

FAILED: Continuous integration, rev:1079
http://jenkins.qa.ubuntu.com/job/webbrowser-app-ci/2011/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-vivid-touch/3515/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/webbrowser-app-vivid-amd64-ci/765/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/webbrowser-app-vivid-armhf-ci/765/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/webbrowser-app-vivid-i386-ci/765/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-vivid-armhf/3512/console

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

review: Needs Fixing (continuous-integration)

lp://qastaging/~uriboni/webbrowser-app/new-tab-wide-format updated on 2015-07-30

1080. By Ugo Riboni on 2015-07-30: Add qt.labs.settings as build dep

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2015-07-30:

#

FAILED: Continuous integration, rev:1080
http://jenkins.qa.ubuntu.com/job/webbrowser-app-ci/2012/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-vivid-touch/3516/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/webbrowser-app-vivid-amd64-ci/766/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/webbrowser-app-vivid-armhf-ci/766/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/webbrowser-app-vivid-i386-ci/766/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-vivid-armhf/3513/console

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

review: Needs Fixing (continuous-integration)

lp://qastaging/~uriboni/webbrowser-app/new-tab-wide-format updated on 2015-07-30

1081. By Ugo Riboni on 2015-07-30: Revert previous commit and actually commit the change to the debian dependencies

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2015-07-30:

#

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

review: Approve (continuous-integration)

Revision history for this message

Olivier Tilloy (osomon) wrote on 2015-07-31:

#

When scrolling the "top sites" view, the first item is drawn on top of the sections header. Either the header should have a higher z-order than the list view, or the list view should be clipped.

The background color of the sections header doesn’t match the visual spec (https://docs.google.com/presentation/d/1ggKmkxUFR5xCBcvkjJ4On9b4iKzEqcycz4hjc16tBGo/edit#slide=id.g2bddb9a1a_038).

In the bookmarks view, when the right panel has active focus the current folder in the left panel should still be highlighted.

In the bookmarks view, the font color for the current bookmark in the right panel shouldn’t be orange.

It seems to be that favicons in the view are too large, and that the font size for delegates is too big as well.

In the visual spec the folders list has a left margin, and folders are separated by horizontal lines.

According to the specification: « User can drag a bookmark in a folder. Bookmarks can only exist in one folder at a time ».

On my vivid desktop, with the UITK staging branch, QmlTests::NewTabLandscapeView::test_switch_sections_by_keyboard() is reliably failing:
  2: FAIL! : QmlTests::NewTabLandscapeView::test_switch_sections_by_keyboard() Compared values are not the same
  2: Actual (): 0
  2: Expected (): 1
  2: Loc: [/home/osomon/dev/phablet/browser/webbrowser-app/tests/unittests/qml/tst_NewTabLandscapeView.qml(191)]

Although not specified, there should probably be scrollbars for all the listviews.

Can you check with design whether we really want all the top sites section to not have a limit? I’d personally add a limit (maybe 10, or 20 top sites max).

Can NewTabLandscapeView be renamed NewTabViewWide?

review: Needs Fixing

lp://qastaging/~uriboni/webbrowser-app/new-tab-wide-format updated on 2015-08-03

1082. By Ugo Riboni on 2015-08-03: Move the Sections block to the bottom of the view so that list items will not overlap it
1083. By Ugo Riboni on 2015-08-03: Keep the current folder selected in orange even when the folder list does not have active focus

Revision history for this message

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

#

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

review: Approve (continuous-integration)

lp://qastaging/~uriboni/webbrowser-app/new-tab-wide-format updated on 2015-08-04

1084. By Ugo Riboni on 2015-08-03: Rename the components to be called *Wide instead of *Landscape
1085. By Ugo Riboni on 2015-08-04: Add scrollbars to all lists
1086. By Ugo Riboni on 2015-08-04: Add a limit of 10 items to the top sites list and unit test it

Revision history for this message

Ugo Riboni (uriboni) wrote on 2015-08-04:

#

> When scrolling the "top sites" view, the first item is drawn on top of the
> sections header. Either the header should have a higher z-order than the list
> view, or the list view should be clipped.

Done

> The background color of the sections header doesn’t match the visual spec (htt
> ps://docs.google.com/presentation/d/1ggKmkxUFR5xCBcvkjJ4On9b4iKzEqcycz4hjc16tB
> Go/edit#slide=id.g2bddb9a1a_038).

This and all the other visuals-related comments you made refer to the above linked document, which is the UX spec, not the visual spec. The current design follows the visual spec as closely as possible (minus some changes that will come from the UITK): https://docs.google.com/presentation/d/1woHjO8K4iqyVZZlfQ4BXL0DhYbwkEmZ7wvcUhYzHDRk/edit?pli=1#slide=id.gb81843c7e_0_42

> According to the specification: « User can drag a bookmark in a folder.
> Bookmarks can only exist in one folder at a time ».

I consulted with design and they need to provide some extra UX and visual guidance. They will do so next week but in the meantime I am adding a provisional implementation based on advice I got from James over IRC.

> On my vivid desktop, with the UITK staging branch,
> QmlTests::NewTabLandscapeView::test_switch_sections_by_keyboard() is reliably
> failing:
> 2: FAIL! :
> QmlTests::NewTabLandscapeView::test_switch_sections_by_keyboard() Compared
> values are not the same
> 2: Actual (): 0
> 2: Expected (): 1
> 2: Loc: [/home/osomon/dev/phablet/browser/webbrowser-
> app/tests/unittests/qml/tst_NewTabLandscapeView.qml(191)]

This is caused by a recent change in the UITK staging branch. I submitted a bug to track it: https://bugs.launchpad.net/ubuntu/+source/ubuntu-ui-toolkit/+bug/1481233

> Although not specified, there should probably be scrollbars for all the
> listviews.

Done

> Can you check with design whether we really want all the top sites section to
> not have a limit? I’d personally add a limit (maybe 10, or 20 top sites max).

James suggested trying with a limit of 10. This is implemented now.

> Can NewTabLandscapeView be renamed NewTabViewWide?

Done

> When scrolling the "top sites" view, the first item is drawn on top of the
> sections header. Either the header should have a higher z-order than the list
> view, or the list view should be clipped.

Done
 
> The background color of the sections header doesn’t match the visual spec (htt
> ps://docs.google.com/presentation/d/1ggKmkxUFR5xCBcvkjJ4On9b4iKzEqcycz4hjc16tB
> Go/edit#slide=id.g2bddb9a1a_038).

This and all the other visuals-related comments you made refer to the above linked document, which is the UX spec, not the visual spec. The current design follows the visual spec as closely as possible (minus some changes that will come from the UITK): https://docs.google.com/presentation/d/1woHjO8K4iqyVZZlfQ4BXL0DhYbwkEmZ7wvcUhYzHDRk/edit?pli=1#slide=id.gb81843c7e_0_42

> According to the specification: « User can drag a bookmark in a folder.
> Bookmarks can only exist in one folder at a time ».

I consulted with design and they need to provide some extra UX and visual guidance. They will do so next week but in the meantime I am adding a provisional implementation based on advice I got from James over IRC.
 
> On my vivid desktop, with the UITK staging branch,
> QmlTests::NewTabLandscapeView::test_switch_sections_by_keyboard() is reliably
> failing:
>   2: FAIL!  :
> QmlTests::NewTabLandscapeView::test_switch_sections_by_keyboard() Compared
> values are not the same
>   2:    Actual   (): 0
>   2:    Expected (): 1
>   2:    Loc: [/home/osomon/dev/phablet/browser/webbrowser-
> app/tests/unittests/qml/tst_NewTabLandscapeView.qml(191)]

This is caused by a recent change in the UITK staging branch. I submitted a bug to track it: https://bugs.launchpad.net/ubuntu/+source/ubuntu-ui-toolkit/+bug/1481233
 
> Although not specified, there should probably be scrollbars for all the
> listviews.

Done
 
> Can you check with design whether we really want all the top sites section to
> not have a limit? I’d personally add a limit (maybe 10, or 20 top sites max).

James suggested trying with a limit of 10. This is implemented now.

> Can NewTabLandscapeView be renamed NewTabViewWide?

Done

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2015-08-04:

#

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

review: Approve (continuous-integration)

lp://qastaging/~uriboni/webbrowser-app/new-tab-wide-format updated on 2015-08-05

1087. By Ugo Riboni on 2015-08-05: Implement a preliminary version of drag and drop support for reordering bookmarks between folders. Visual and UX design input missing so there are obvious placeholders.
1088. By Ugo Riboni on 2015-08-05: Merge changes from trunk
1089. By Ugo Riboni on 2015-08-05: Correctly honor draggable property

Revision history for this message

Olivier Tilloy (osomon) wrote on 2015-08-05:

#

Why is there a new "selectedIndexNewTabViewWide" setting? If, as I suspect, this is because the UX spec mandates that the state being retained, then it shouldn’t be a setting. Instead, the StateSaver should be used for that.

Favicons appear to be too large compared to the visual spec (hint: the Favicon component already has a built-in default size, you should probably not change it).

Folder names in bookmarks (left panel) shouldn’t be black when not selected, in the visual spec they appear to be darkGrey.

The spec doesn’t consider the case where a bookmark folder is empty. Can you check with design whether we should display some sort of informative message in this case, or if having an empty right panel is expected?

Can the failing unit test be temporarily skipped (http://doc.qt.io/qt-5/qml-qttest-testcase.html#skip-method) with a reference to bug #1481233 ?

If I swipe a bookmark to the right to delete it, the red delete action covers the left panel.

Bookmark dnd doesn’t seem to work here (I’m not seeing any icon appear when hovering over a bookmark). Might be due to me running a locally-built (and not installed) version of the UITK staging branch.

43 + Binding { target: newTabViewLoader.item; property: "focus"; value: newTabViewLoader.focus }
Is this really needed? A Loader is a FocusScope, so I would expect that always setting focus to true in the component being loaded would be enough to have focus handling just work.

Why was asynchronous loading removed from newTabViewLoader?

There’s now two strings for top sites: "Top Sites" and "Top sites". Can this be made consistent?

In bookmarks-folderlist-model.[h|cpp], I don’t think the count() method is necessary, you can simply define the getter for the count property to be rowCount(). And countChanged() doesn’t appear to ever be emitted, that seems wrong.

review: Needs Fixing

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2015-08-05:

#

FAILED: Continuous integration, rev:1087
http://jenkins.qa.ubuntu.com/job/webbrowser-app-ci/2032/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-vivid-touch/3583/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/webbrowser-app-vivid-amd64-ci/786/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/webbrowser-app-vivid-armhf-ci/786/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/webbrowser-app-vivid-i386-ci/786/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-vivid-armhf/3580/console

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

review: Needs Fixing (continuous-integration)

Revision history for this message

Olivier Tilloy (osomon) wrote on 2015-08-05:

#

I am now seeing the DND icon when hovering over a bookmark, and I can drag it and drop it in another folder, but this doesn’t appear to actually move it.

review: Needs Fixing

lp://qastaging/~uriboni/webbrowser-app/new-tab-wide-format updated on 2015-08-05

1090. By Ugo Riboni on 2015-08-05: Fix incorrect item reference that prevented dropping bookmarks to work

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2015-08-05:

#

FAILED: Continuous integration, rev:1089
http://jenkins.qa.ubuntu.com/job/webbrowser-app-ci/2033/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-vivid-touch/3587/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/webbrowser-app-vivid-amd64-ci/787/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/webbrowser-app-vivid-armhf-ci/787/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/webbrowser-app-vivid-i386-ci/787/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-vivid-armhf/3584/console

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

review: Needs Fixing (continuous-integration)

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2015-08-05:

#

FAILED: Continuous integration, rev:1090
http://jenkins.qa.ubuntu.com/job/webbrowser-app-ci/2035/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-vivid-touch/3589/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/webbrowser-app-vivid-amd64-ci/789/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/webbrowser-app-vivid-armhf-ci/789/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/webbrowser-app-vivid-i386-ci/789/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-vivid-armhf/3586/console

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

review: Needs Fixing (continuous-integration)

Revision history for this message

Olivier Tilloy (osomon) wrote on 2015-08-06:

#

In debian/control, the build dependency and runtime dependency of webbrowser-app on qtdeclarative5-ubuntu-ui-toolkit-plugin need to specify that they require a version >= 1.3.

review: Needs Fixing

lp://qastaging/~uriboni/webbrowser-app/new-tab-wide-format updated on 2015-08-06

1091. By Ugo Riboni on 2015-08-05: Use default size for Favicons in delegates
1092. By Ugo Riboni on 2015-08-05: Adjust folder color to match visual spec
1093. By Ugo Riboni on 2015-08-05: More fixes on the drag and drop
1094. By Ugo Riboni on 2015-08-05: Clip the lists to prevent the slide-to-delete element to cover folders
1095. By Ugo Riboni on 2015-08-05: Remove unnecessary binding for keyboard focus
1096. By Ugo Riboni on 2015-08-05: Remove async loading on the new tab view loader as it was removed by mistake by original author of the branch
1097. By Ugo Riboni on 2015-08-05: Fix case of "top sites" string to match the non-widescreen version
1098. By Ugo Riboni on 2015-08-05: More properly implement the count property of the folder list model
1099. By Ugo Riboni on 2015-08-06: Rename the settings key used to store the default section to something more clear
1100. By Ugo Riboni on 2015-08-06: Clip list items instead of lists, so that we can still drag the items without having to reparent them to the outer view
1101. By Ugo Riboni on 2015-08-06: Fix unit tests broken by previous change
1102. By Ugo Riboni on 2015-08-06: Skip test that would fail due to UITK bug
1103. By Ugo Riboni on 2015-08-06: Add a (currently failing) unit test for drag and drop, and fix a mistake in how the drag start event was being emitted

Revision history for this message

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

#

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

review: Approve (continuous-integration)

Revision history for this message

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

#

FAILED: Continuous integration, rev:1103
http://jenkins.qa.ubuntu.com/job/webbrowser-app-ci/2039/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-vivid-touch/3599/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/webbrowser-app-vivid-amd64-ci/793/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/webbrowser-app-vivid-armhf-ci/793/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/webbrowser-app-vivid-i386-ci/793/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-vivid-armhf/3596/console

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

review: Needs Fixing (continuous-integration)

lp://qastaging/~uriboni/webbrowser-app/new-tab-wide-format updated on 2015-08-06

1104. By Ugo Riboni on 2015-08-06: Reimplement the drag and drop test as autopilot tests

Revision history for this message

Ugo Riboni (uriboni) wrote on 2015-08-06:

#

All other items that are not specifically mentioned here should have been fixed or addressed.

> Why is there a new "selectedIndexNewTabViewWide" setting? If, as I suspect,
> this is because the UX spec mandates that the state being retained, then it
> shouldn’t be a setting. Instead, the StateSaver should be used for that.

It needs a setting because the StateSaver won't save the state when closing the app normally, and we want to remember in which section to open the view by default regardless of how the app was closed.

> The spec doesn’t consider the case where a bookmark folder is empty. Can you
> check with design whether we should display some sort of informative message
> in this case, or if having an empty right panel is expected?

I had already asked some days ago. They are working on it AFAIK.

Revision history for this message

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

#

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

review: Approve (continuous-integration)

Revision history for this message

Olivier Tilloy (osomon) wrote on 2015-08-06:

#

If I swipe a bookmark to the right to delete it, hovering over it still displays the grip that allows dragging it to a different folder (partially clipped by the window). I would expect this grip to be disabled/hidden while awaiting confirmation of whether to delete or not.

If I open a new tab, then open the history view (either from the drawer menu or with Ctrl+H), then close it, the sections header’s rendering is busted (the background color is drawn, but not the sections). It appears to be a focus issue, as it fixes itself if I press twice the down arrow key to focus the view again. This doesn’t happen with e.g. the settings view.

As pointed out in an earlier comment, in debian/control, the build dependency and runtime dependency of webbrowser-app on qtdeclarative5-ubuntu-ui-toolkit-plugin need to specify that they require a version >= 1.3.

When running the autopilot tests on my vivid desktop with version 1.3 of the toolkit, I need to modify line 295 of tests/autopilot/webbrowser_app/emulators/browser.py to make the tests pass: s/Icon11/Icon/. I was told by Brendan earlier today that this is a recent change in autopilot, can you apply it in this branch and verify that CI is happy with it?

Even with the above fixed, I’m seeing 3 errors when running the autopilot tests for test_new_tab_view on my desktop:
  webbrowser_app.tests.test_new_tab_view.TestNewTabViewContents.test_open_top_site
  webbrowser_app.tests.test_new_tab_view.TestNewTabViewContents.test_default_home_bookmark
  webbrowser_app.tests.test_new_tab_view.TestNewTabViewContents.test_open_bookmark

review: Needs Fixing

Revision history for this message

Olivier Tilloy (osomon) wrote on 2015-08-06:

#

Regarding drag and drop in unit tests: I just did a quick test, modifying test_drag() to do the following:

    function log_item_pos() { console.log("item.pos =", item.x, item.y) }
    log_item_pos()
    item.xChanged.connect(log_item_pos)
    item.yChanged.connect(log_item_pos)
    var c = centerOf(grip)
    mouseDrag(grip, c.x, c.y, -200, 100)

When running it, even though I’m not visualizing the item move, logging proves that it’s actually moving:

    qml: item.pos = 0 56
    qml: item.pos = -11 78
    qml: item.pos = -11 78
    qml: item.pos = -22 133
    qml: item.pos = -22 133
    qml: item.pos = -233 222
    qml: item.pos = -233 222
    qml: item.pos = 0 222
    qml: item.pos = 0 56

lp://qastaging/~uriboni/webbrowser-app/new-tab-wide-format updated on 2015-08-10

1105. By Ugo Riboni on 2015-08-10: Bump the UITK import version in debian/control
1106. By Ugo Riboni on 2015-08-10: Hide the dragging grip on bookmarks while the slide-to-delete action in progress
1107. By Ugo Riboni on 2015-08-10: Move some tests to the non-widescreen class only as the same tests are already implemented as unittests for the widescreen version and would fail in any case
1108. By Ugo Riboni on 2015-08-10: Fix a problem in how Icons are addressed in AP tests that was making some of them fail when using the uitk 1.3
1109. By Ugo Riboni on 2015-08-10: Prevent the Sections component to behave erratically when componets from 1.2 are imported while it is displayed. The real fix is to replace all imports to 1.3 but this suffices for now, while the work is being done in another branch.

Revision history for this message

Ugo Riboni (uriboni) wrote on 2015-08-10:

#

Fixed unless otherwise noted

> If I open a new tab, then open the history view (either from the drawer menu
> or with Ctrl+H), then close it, the sections header’s rendering is busted (the
> background color is drawn, but not the sections). It appears to be a focus
> issue, as it fixes itself if I press twice the down arrow key to focus the
> view again. This doesn’t happen with e.g. the settings view.

This is caused by the HistoryView importing Ubuntu.Components 1.2
According to the SDK team mixing imports seems to be not supported anymore. I fixed this only in HistoryView for now but the right thing to do would be to bump all imports to 1.3 to avoid other unexpected and hard to debug problems.

> When running the autopilot tests on my vivid desktop with version 1.3 of the
> toolkit, I need to modify line 295 of
> tests/autopilot/webbrowser_app/emulators/browser.py to make the tests pass:
> s/Icon11/Icon/. I was told by Brendan earlier today that this is a recent
> change in autopilot, can you apply it in this branch and verify that CI is
> happy with it?

Let's see what CI says.

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2015-08-10:

#

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

review: Needs Fixing (continuous-integration)

lp://qastaging/~uriboni/webbrowser-app/new-tab-wide-format updated on 2015-08-11

1110. By Ugo Riboni on 2015-08-11: Merge changes from trunk
1111. By Ugo Riboni on 2015-08-11: Bump import to 2.4 in newly added unit test file

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2015-08-11:

#

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

review: Needs Fixing (continuous-integration)

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2015-08-11:

#

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

review: Needs Fixing (continuous-integration)

Revision history for this message

Olivier Tilloy (osomon) wrote on 2015-08-11:

#

The 'inBookmarksView' property should be marked readonly.

Ctrl+Tab works to switch between sections, Tab doesn’t, even though there seems to be code for handling this (is it bug #1481233 ?).

If I cancel a drag (by dropping it somewhere it’s not accepted), it seems the bookmarks list’s interactivity is not restored (I cannot click and drag to move up/down the list).

review: Needs Fixing

lp://qastaging/~uriboni/webbrowser-app/new-tab-wide-format updated on 2015-08-11

1112. By Ugo Riboni on 2015-08-11: Merge changes from trunk
1113. By Ugo Riboni on 2015-08-11: Correctly mark a property as readonly
1114. By Ugo Riboni on 2015-08-11: Fix a bug preventing the interactivity in the boomarks list to be correctly restored at the end of a drag

Revision history for this message

Ugo Riboni (uriboni) wrote on 2015-08-11:

#

All fixed and yes, the problem with TAB is caused by bug #1481233

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2015-08-11:

#

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

review: Needs Fixing (continuous-integration)

lp://qastaging/~uriboni/webbrowser-app/new-tab-wide-format updated on 2015-08-11

1115. By Ugo Riboni on 2015-08-11: More fixes for correctly emitting the end of drag event
1116. By Ugo Riboni on 2015-08-11: Make it impossible to have keyboard focus within an empty list

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2015-08-11:

#

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

review: Needs Fixing (continuous-integration)

Revision history for this message

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

#

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

review: Needs Fixing (continuous-integration)

Revision history for this message

Olivier Tilloy (osomon) wrote on 2015-08-12:

#

I’m reliably getting 4 autopilot test failures on my desktop:

webbrowser_app.tests.test_private.TestPrivateView.test_url_showing_in_top_sites_in_and_out_private_mode
webbrowser_app.tests.test_bookmark_options.TestBookmarkOptions.test_save_bookmarked_url_in_new_folder
webbrowser_app.tests.test_bookmark_options.TestBookmarkOptions.test_save_bookmarked_url_in_existing_folder
webbrowser_app.tests.test_bookmark_options.TestBookmarkOptions.test_save_bookmarked_url_in_default_folder

review: Needs Fixing

lp://qastaging/~uriboni/webbrowser-app/new-tab-wide-format updated on 2015-08-12

1117. By Ugo Riboni on 2015-08-12: Fix some recent AP tests by smoothing away the differences between the wide and narrow new tab views via emulators and helper functions
1118. By Ugo Riboni on 2015-08-12: Remove method from main window emulator as it does not belong there
1119. By Ugo Riboni on 2015-08-12: Fix flake8

Revision history for this message

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

#

FAILED: Continuous integration, rev:1117
http://jenkins.qa.ubuntu.com/job/webbrowser-app-ci/2073/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-vivid-touch/3669/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/webbrowser-app-vivid-amd64-ci/827/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/webbrowser-app-vivid-armhf-ci/827/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/webbrowser-app-vivid-i386-ci/827/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-vivid-armhf/3666/console

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

review: Needs Fixing (continuous-integration)

Revision history for this message

Olivier Tilloy (osomon) wrote on 2015-08-12:

#

Still seeing 3 autopilot failures on desktop:

webbrowser_app.tests.test_bookmark_options.TestBookmarkOptions.test_save_bookmarked_url_in_existing_folder
webbrowser_app.tests.test_new_tab_view.TestNewTabViewContentsWide.test_remove_top_sites
webbrowser_app.tests.test_private.TestPrivateView.test_url_showing_in_top_sites_in_and_out_private_mode

review: Needs Fixing

lp://qastaging/~uriboni/webbrowser-app/new-tab-wide-format updated on 2015-08-12

1120. By Ugo Riboni on 2015-08-12: Refactor to fix a failing test
1121. By Ugo Riboni on 2015-08-12: Ensure correct section is always selected, not taking for granted the default
1122. By Ugo Riboni on 2015-08-12: More fixes for tests in narrow screen mode

Revision history for this message

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

#

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

review: Needs Fixing (continuous-integration)

Revision history for this message

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

#

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

review: Needs Fixing (continuous-integration)

Revision history for this message

Olivier Tilloy (osomon) wrote on 2015-08-12:

#

This looks good now.

review: Approve

webbrowser-app

Merge lp://qastaging/~uriboni/webbrowser-app/new-tab-wide-format into lp://qastaging/webbrowser-app

Commit message

Description of the change

Preview Diff

Subscribers