Merge lp:~abreu-alexandre/webbrowser-app/remove-open-in-browser-button-in-popups into lp:webbrowser-app

Proposed by Alexandre Abreu
Status: Approved
Approved by: Alberto Mardegan
Approved revision: 1503
Proposed branch: lp:~abreu-alexandre/webbrowser-app/remove-open-in-browser-button-in-popups
Merge into: lp:webbrowser-app
Diff against target: 81 lines (+26/-3)
3 files modified
src/app/webcontainer/PopupWindowController.qml (+9/-2)
src/app/webcontainer/PopupWindowOverlay.qml (+2/-1)
tests/autopilot/webapp_container/tests/test_popup_webview_overlay.py (+15/-0)
To merge this branch: bzr merge lp:~abreu-alexandre/webbrowser-app/remove-open-in-browser-button-in-popups
Reviewer Review Type Date Requested Status
Alberto Mardegan (community) Approve
system-apps-ci-bot continuous-integration Needs Fixing
Review via email: mp+300633@code.launchpad.net

Commit message

Remove the "open in the webrowser" button in an Overlay created as Popups

Description of the change

Remove the "open in the webrowser" button in an Overlay created as Popups

To post a comment you must log in.
Revision history for this message
system-apps-ci-bot (system-apps-ci-bot) wrote :

FAILED: Continuous integration, rev:1502
https://jenkins.canonical.com/system-apps/job/lp-webbrowser-app-ci/586/
Executed test runs:
    FAILURE: https://jenkins.canonical.com/system-apps/job/build/1010/console
    SUCCESS: https://jenkins.canonical.com/system-apps/job/build-0-fetch/1010
    SUCCESS: https://jenkins.canonical.com/system-apps/job/build-1-sourcepkg/release=vivid+overlay/908
    SUCCESS: https://jenkins.canonical.com/system-apps/job/build-1-sourcepkg/release=xenial+overlay/908
    SUCCESS: https://jenkins.canonical.com/system-apps/job/build-1-sourcepkg/release=yakkety/908
    FAILURE: https://jenkins.canonical.com/system-apps/job/build-2-binpkg/arch=amd64,release=vivid+overlay/902/console
    FAILURE: https://jenkins.canonical.com/system-apps/job/build-2-binpkg/arch=amd64,release=xenial+overlay/902/console
    SUCCESS: https://jenkins.canonical.com/system-apps/job/build-2-binpkg/arch=amd64,release=yakkety/902
        deb: https://jenkins.canonical.com/system-apps/job/build-2-binpkg/arch=amd64,release=yakkety/902/artifact/output/*zip*/output.zip
    SUCCESS: https://jenkins.canonical.com/system-apps/job/build-2-binpkg/arch=armhf,release=vivid+overlay/902
        deb: https://jenkins.canonical.com/system-apps/job/build-2-binpkg/arch=armhf,release=vivid+overlay/902/artifact/output/*zip*/output.zip
    SUCCESS: https://jenkins.canonical.com/system-apps/job/build-2-binpkg/arch=armhf,release=xenial+overlay/902
        deb: https://jenkins.canonical.com/system-apps/job/build-2-binpkg/arch=armhf,release=xenial+overlay/902/artifact/output/*zip*/output.zip
    SUCCESS: https://jenkins.canonical.com/system-apps/job/build-2-binpkg/arch=armhf,release=yakkety/902
        deb: https://jenkins.canonical.com/system-apps/job/build-2-binpkg/arch=armhf,release=yakkety/902/artifact/output/*zip*/output.zip
    FAILURE: https://jenkins.canonical.com/system-apps/job/build-2-binpkg/arch=i386,release=vivid+overlay/902/console
    SUCCESS: https://jenkins.canonical.com/system-apps/job/build-2-binpkg/arch=i386,release=xenial+overlay/902
        deb: https://jenkins.canonical.com/system-apps/job/build-2-binpkg/arch=i386,release=xenial+overlay/902/artifact/output/*zip*/output.zip
    SUCCESS: https://jenkins.canonical.com/system-apps/job/build-2-binpkg/arch=i386,release=yakkety/902
        deb: https://jenkins.canonical.com/system-apps/job/build-2-binpkg/arch=i386,release=yakkety/902/artifact/output/*zip*/output.zip

Click here to trigger a rebuild:
https://jenkins.canonical.com/system-apps/job/lp-webbrowser-app-ci/586/rebuild

review: Needs Fixing (continuous-integration)
Revision history for this message
system-apps-ci-bot (system-apps-ci-bot) wrote :

FAILED: Continuous integration, rev:1502
https://jenkins.canonical.com/system-apps/job/lp-webbrowser-app-ci/587/
Executed test runs:
    SUCCESS: https://jenkins.canonical.com/system-apps/job/build/1011
    FAILURE: https://jenkins.canonical.com/system-apps/job/test-0-autopkgtest/label=phone-armhf,release=vivid+overlay,testname=default/190/console
    SUCCESS: https://jenkins.canonical.com/system-apps/job/build-0-fetch/1011
    SUCCESS: https://jenkins.canonical.com/system-apps/job/build-1-sourcepkg/release=vivid+overlay/909
    SUCCESS: https://jenkins.canonical.com/system-apps/job/build-1-sourcepkg/release=xenial+overlay/909
    SUCCESS: https://jenkins.canonical.com/system-apps/job/build-1-sourcepkg/release=yakkety/909
    SUCCESS: https://jenkins.canonical.com/system-apps/job/build-2-binpkg/arch=amd64,release=vivid+overlay/903
        deb: https://jenkins.canonical.com/system-apps/job/build-2-binpkg/arch=amd64,release=vivid+overlay/903/artifact/output/*zip*/output.zip
    SUCCESS: https://jenkins.canonical.com/system-apps/job/build-2-binpkg/arch=amd64,release=xenial+overlay/903
        deb: https://jenkins.canonical.com/system-apps/job/build-2-binpkg/arch=amd64,release=xenial+overlay/903/artifact/output/*zip*/output.zip
    SUCCESS: https://jenkins.canonical.com/system-apps/job/build-2-binpkg/arch=amd64,release=yakkety/903
        deb: https://jenkins.canonical.com/system-apps/job/build-2-binpkg/arch=amd64,release=yakkety/903/artifact/output/*zip*/output.zip
    SUCCESS: https://jenkins.canonical.com/system-apps/job/build-2-binpkg/arch=armhf,release=vivid+overlay/903
        deb: https://jenkins.canonical.com/system-apps/job/build-2-binpkg/arch=armhf,release=vivid+overlay/903/artifact/output/*zip*/output.zip
    SUCCESS: https://jenkins.canonical.com/system-apps/job/build-2-binpkg/arch=armhf,release=xenial+overlay/903
        deb: https://jenkins.canonical.com/system-apps/job/build-2-binpkg/arch=armhf,release=xenial+overlay/903/artifact/output/*zip*/output.zip
    SUCCESS: https://jenkins.canonical.com/system-apps/job/build-2-binpkg/arch=armhf,release=yakkety/903
        deb: https://jenkins.canonical.com/system-apps/job/build-2-binpkg/arch=armhf,release=yakkety/903/artifact/output/*zip*/output.zip
    SUCCESS: https://jenkins.canonical.com/system-apps/job/build-2-binpkg/arch=i386,release=vivid+overlay/903
        deb: https://jenkins.canonical.com/system-apps/job/build-2-binpkg/arch=i386,release=vivid+overlay/903/artifact/output/*zip*/output.zip
    SUCCESS: https://jenkins.canonical.com/system-apps/job/build-2-binpkg/arch=i386,release=xenial+overlay/903
        deb: https://jenkins.canonical.com/system-apps/job/build-2-binpkg/arch=i386,release=xenial+overlay/903/artifact/output/*zip*/output.zip
    SUCCESS: https://jenkins.canonical.com/system-apps/job/build-2-binpkg/arch=i386,release=yakkety/903
        deb: https://jenkins.canonical.com/system-apps/job/build-2-binpkg/arch=i386,release=yakkety/903/artifact/output/*zip*/output.zip

Click here to trigger a rebuild:
https://jenkins.canonical.com/system-apps/job/lp-webbrowser-app-ci/587/rebuild

review: Needs Fixing (continuous-integration)
Revision history for this message
Alberto Mardegan (mardy) wrote :

LGTM, but I'd suggest a minor change to improve readability (see inline comment)

review: Needs Fixing
1503. By Alexandre Abreu

Improve overlay open in browser button visibility predicate's readability

Revision history for this message
system-apps-ci-bot (system-apps-ci-bot) wrote :

FAILED: Continuous integration, rev:1503
https://jenkins.canonical.com/system-apps/job/lp-webbrowser-app-ci/590/
Executed test runs:
    FAILURE: https://jenkins.canonical.com/system-apps/job/build/1054/console
    SUCCESS: https://jenkins.canonical.com/system-apps/job/build-0-fetch/1054
    SUCCESS: https://jenkins.canonical.com/system-apps/job/build-1-sourcepkg/release=vivid+overlay/951
    SUCCESS: https://jenkins.canonical.com/system-apps/job/build-1-sourcepkg/release=xenial+overlay/951
    SUCCESS: https://jenkins.canonical.com/system-apps/job/build-1-sourcepkg/release=yakkety/951
    SUCCESS: https://jenkins.canonical.com/system-apps/job/build-2-binpkg/arch=amd64,release=vivid+overlay/942
        deb: https://jenkins.canonical.com/system-apps/job/build-2-binpkg/arch=amd64,release=vivid+overlay/942/artifact/output/*zip*/output.zip
    SUCCESS: https://jenkins.canonical.com/system-apps/job/build-2-binpkg/arch=amd64,release=xenial+overlay/942
        deb: https://jenkins.canonical.com/system-apps/job/build-2-binpkg/arch=amd64,release=xenial+overlay/942/artifact/output/*zip*/output.zip
    FAILURE: https://jenkins.canonical.com/system-apps/job/build-2-binpkg/arch=amd64,release=yakkety/942/console
    SUCCESS: https://jenkins.canonical.com/system-apps/job/build-2-binpkg/arch=armhf,release=vivid+overlay/942
        deb: https://jenkins.canonical.com/system-apps/job/build-2-binpkg/arch=armhf,release=vivid+overlay/942/artifact/output/*zip*/output.zip
    SUCCESS: https://jenkins.canonical.com/system-apps/job/build-2-binpkg/arch=armhf,release=xenial+overlay/942
        deb: https://jenkins.canonical.com/system-apps/job/build-2-binpkg/arch=armhf,release=xenial+overlay/942/artifact/output/*zip*/output.zip
    SUCCESS: https://jenkins.canonical.com/system-apps/job/build-2-binpkg/arch=armhf,release=yakkety/942
        deb: https://jenkins.canonical.com/system-apps/job/build-2-binpkg/arch=armhf,release=yakkety/942/artifact/output/*zip*/output.zip
    SUCCESS: https://jenkins.canonical.com/system-apps/job/build-2-binpkg/arch=i386,release=vivid+overlay/942
        deb: https://jenkins.canonical.com/system-apps/job/build-2-binpkg/arch=i386,release=vivid+overlay/942/artifact/output/*zip*/output.zip
    SUCCESS: https://jenkins.canonical.com/system-apps/job/build-2-binpkg/arch=i386,release=xenial+overlay/942
        deb: https://jenkins.canonical.com/system-apps/job/build-2-binpkg/arch=i386,release=xenial+overlay/942/artifact/output/*zip*/output.zip
    FAILURE: https://jenkins.canonical.com/system-apps/job/build-2-binpkg/arch=i386,release=yakkety/942/console

Click here to trigger a rebuild:
https://jenkins.canonical.com/system-apps/job/lp-webbrowser-app-ci/590/rebuild

review: Needs Fixing (continuous-integration)
Revision history for this message
Alberto Mardegan (mardy) wrote :

LGTM!

review: Approve

Unmerged revisions

1503. By Alexandre Abreu

Improve overlay open in browser button visibility predicate's readability

1502. By Alexandre Abreu

add tests

1501. By Alexandre Abreu

draft

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'src/app/webcontainer/PopupWindowController.qml'
2--- src/app/webcontainer/PopupWindowController.qml 2016-03-24 14:06:22 +0000
3+++ src/app/webcontainer/PopupWindowController.qml 2016-07-27 15:37:37 +0000
4@@ -167,7 +167,10 @@
5 view.opacity = visible ? 1.0 : 0.0
6 }
7 }
8- function createPopupView(parentView, params, isRequestFromMainWebappWebview, context) {
9+ function createPopupView(parentView,
10+ params,
11+ isRequestFromMainWebappWebview,
12+ context) {
13 var view = popupWebOverlayFactory.createObject(
14 parentView,
15 params);
16@@ -194,9 +197,13 @@
17 handleNewViewAdded(view)
18 newViewCreated(view.url)
19 }
20- function createPopupViewForRequest(parentView, request, isRequestFromMainWebappWebview, context) {
21+ function createPopupViewForRequest(parentView,
22+ request,
23+ isRequestFromMainWebappWebview,
24+ context) {
25 createPopupView(parentView,
26 { request: request,
27+ disposition: request.disposition,
28 webContext: context,
29 popupWindowController: controller,
30 mediaAccessDialogComponent: mediaAccessDialogComponent
31
32=== modified file 'src/app/webcontainer/PopupWindowOverlay.qml'
33--- src/app/webcontainer/PopupWindowOverlay.qml 2016-06-17 17:39:58 +0000
34+++ src/app/webcontainer/PopupWindowOverlay.qml 2016-07-27 15:37:37 +0000
35@@ -28,6 +28,7 @@
36
37 property var popupWindowController
38 property var webContext
39+ property var disposition
40 property alias currentWebview: popupWebview
41 property alias request: popupWebview.request
42 property alias url: popupWebview.url
43@@ -147,7 +148,7 @@
44 iconSize: 0.6 * height
45
46 enabled: true
47- visible: true
48+ visible: popup.disposition !== Oxide.NavigationRequest.DispositionNewPopup
49
50 MouseArea {
51 anchors.fill: parent
52
53=== modified file 'tests/autopilot/webapp_container/tests/test_popup_webview_overlay.py'
54--- tests/autopilot/webapp_container/tests/test_popup_webview_overlay.py 2016-04-25 16:07:15 +0000
55+++ tests/autopilot/webapp_container/tests/test_popup_webview_overlay.py 2016-07-27 15:37:37 +0000
56@@ -71,6 +71,12 @@
57 overlay.select_single(objectName="overlayWebview").url,
58 Contains('/open-close-content'))
59
60+ open_in_browser_button = overlay.select_single(
61+ objectName="overlayButtonOpenInBrowser")
62+ self.assertThat(
63+ open_in_browser_button.visible,
64+ Equals(True))
65+
66 self.assertThat(
67 lambda: animation_watcher.num_emissions,
68 Eventually(GreaterThan(animation_signal_emission)))
69@@ -187,3 +193,12 @@
70 self.assertThat(
71 lambda: len(self.get_popup_overlay_views()),
72 Eventually(Equals(overlay_opened_from_main_view_count)))
73+
74+ overlays = self.get_popup_overlay_views()
75+ for overlay in overlays:
76+ open_in_browser_button = overlay.select_single(
77+ objectName="overlayButtonOpenInBrowser")
78+
79+ self.assertThat(
80+ open_in_browser_button.visible,
81+ Equals(False))

Subscribers

People subscribed via source and target branches

to status/vote changes: