Merge into trunk : add-ap-tests-for-navigation-and-popup : Code : webbrowser-app

Status:	Needs review
Proposed branch:	lp:~abreu-alexandre/webbrowser-app/add-ap-tests-for-navigation-and-popup
Merge into:	lp:webbrowser-app
Diff against target:	564 lines (+319/-12) 11 files modified src/app/webcontainer/WebApp.qml (+2/-0) src/app/webcontainer/WebViewImplOxide.qml (+35/-9) src/app/webcontainer/WebViewImplWebkit.qml (+3/-0) src/app/webcontainer/WebappContainerWebview.qml (+4/-1) src/app/webcontainer/webapp-container.cpp (+24/-2) src/app/webcontainer/webapp-container.h (+1/-0) src/app/webcontainer/webapp-container.qml (+2/-0) tests/autopilot/webapp_container/tests/__init__.py (+14/-0) tests/autopilot/webapp_container/tests/fake_servers.py (+84/-0) tests/autopilot/webapp_container/tests/test_navigation_filtering.py (+92/-0) tests/autopilot/webapp_container/tests/test_navigation_popup.py (+58/-0)
To merge this branch:	bzr merge lp:~abreu-alexandre/webbrowser-app/add-ap-tests-for-navigation-and-popup
Related bugs:	Link a bug report

Reviewer	Review Type	Date Requested	Status
system-apps-ci-bot	continuous-integration		Needs Fixing on 2016-05-10
PS Jenkins bot	continuous-integration		Approve on 2014-07-09
Ubuntu Phablet Team		2014-04-30	Pending
Review via email: mp+217811@code.launchpad.net

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2014-04-30:

#

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

review: Needs Fixing (continuous-integration)

Reply

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2014-05-02:

#

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

review: Needs Fixing (continuous-integration)

Reply

lp:~abreu-alexandre/webbrowser-app/add-ap-tests-for-navigation-and-popup updated on 2014-05-31

509. By Olivier Tilloy on 2014-05-05: Work around a recent regression by forcing the OSK to show up when the address bar is being focused. Fixes: 1316057
510. By PS Jenkins bot on 2014-05-05: Releasing 0.23+14.10.20140505-0ubuntu1
511. By Michael Sheldon on 2014-05-05: Resolve image URLs beginning with a double slash correctly for context menu items Fixes: 1311626
512. By Adnane Belmadiaf on 2014-05-05: Enabled passwordEchoEnabled Fixes: 1314251
513. By Olivier Tilloy on 2014-05-05: Build the models in a separate static lib, and link the unit tests against it.
This speeds up build time by avoiding having to recompile the models’ source for each unit test.
514. By Olivier Tilloy on 2014-05-05: Handle javascript console messages.
515. By Olivier Tilloy on 2014-05-05: Escape literal dots in UA override matching regular expressions.
516. By Olivier Tilloy on 2014-05-05: Enable localStorage by default in the browser. Fixes: 1309673, 1310658
517. By Olivier Tilloy on 2014-05-05: Ensure that the URL actually changes so that the address bar is updated in case the user has entered a new address that redirects to where she previously was. Fixes: 1306615
518. By Olivier Tilloy on 2014-05-05: Update bzr ignore rules.
519. By Alberto Mardegan on 2014-05-05: Split UbuntuWebContext into two different components:
- UbuntuWebContext, which is a WebContext derivative with the UA overrides for Ubuntu
- UbuntuSharedWebContext, which is a singleton for UbuntuWebContext
520. By PS Jenkins bot on 2014-05-05: Releasing 0.23+14.10.20140505.1-0ubuntu1
521. By Launchpad Translations on behalf of phablet-team on 2014-05-06: Launchpad automatic translations update.
522. By Olivier Tilloy on 2014-05-06: Enable cross compilation for an ARM target on an X86 host.
523. By Olivier Tilloy on 2014-05-06: Port autopilot tests to Python 3.
524. By PS Jenkins bot on 2014-05-06: Releasing 0.23+14.10.20140506-0ubuntu1
525. By Alexandre Abreu on 2014-05-06: Add capability for single webapps to have specific UA overrides for the website that they serve Fixes: 1245465
526. By PS Jenkins bot on 2014-05-06: Releasing 0.23+14.10.20140506.1-0ubuntu1
527. By Launchpad Translations on behalf of phablet-team on 2014-05-07: Launchpad automatic translations update.
528. By Launchpad Translations on behalf of phablet-team on 2014-05-12: Launchpad automatic translations update.
529. By Alexandre Abreu on 2014-05-14: Add --local-webapp-manifest webapp container cli option to simplify the command line in the case of a local manifest.json file definition. This is to become a bit more important now that the manifest support thing like ua overrides that would be beneficial to webapp on touch.
530. By PS Jenkins bot on 2014-05-14: Releasing 0.23+14.10.20140514.1-0ubuntu1
531. By Michael Sheldon on 2014-05-15: Add support for downloading images via download manager and content-hub on non-desktop platforms.
532. By Tim Peeters on 2014-05-15: Push the initial Page on the PageStack.
533. By PS Jenkins bot on 2014-05-15: Releasing 0.23+14.10.20140515.1-0ubuntu1
534. By Launchpad Translations on behalf of phablet-team on 2014-05-16: Launchpad automatic translations update.
535. By Launchpad Translations on behalf of phablet-team on 2014-05-17: Launchpad automatic translations update.
536. By Launchpad Translations on behalf of phablet-team on 2014-05-18: Launchpad automatic translations update.
537. By Launchpad Translations on behalf of phablet-team on 2014-05-19: Launchpad automatic translations update.
538. By Olivier Tilloy on 2014-05-21: Remove an extraneous whitespace in the default UA on mobile.
539. By PS Jenkins bot on 2014-05-21: Releasing 0.23+14.10.20140521-0ubuntu1
540. By Olivier Tilloy on 2014-05-21: Fix FTBFS with Qt 5.3. Fixes: 1321440
541. By PS Jenkins bot on 2014-05-21: Releasing 0.23+14.10.20140521.1-0ubuntu1
542. By Launchpad Translations on behalf of phablet-team on 2014-05-22: Launchpad automatic translations update.
543. By Olivier Tilloy on 2014-05-22: Various optimizations to the activity view. Fixes: 1260980
544. By Olivier Tilloy on 2014-05-22: Do not override the default height of the TextField that serves as the address bar. Fixes: 1317866
545. By PS Jenkins bot on 2014-05-22: Releasing 0.23+14.10.20140522.1-0ubuntu1
546. By Launchpad Translations on behalf of phablet-team on 2014-05-23: Launchpad automatic translations update.
547. By Launchpad Translations on behalf of phablet-team on 2014-05-24: Launchpad automatic translations update.
548. By Olivier Tilloy on 2014-05-26: Ensure the current webview is hidden while the activity view is visible,
and work around a bug in oxide that prevented new empty tabs from rendering.
549. By PS Jenkins bot on 2014-05-26: Releasing 0.23+14.10.20140526-0ubuntu1
550. By Olivier Tilloy on 2014-05-26: Ensure the main page that contains the webviews fills the main view. Fixes: 1321462
551. By PS Jenkins bot on 2014-05-26: Releasing 0.23+14.10.20140526.1-0ubuntu1
552. By Launchpad Translations on behalf of phablet-team on 2014-05-27: Launchpad automatic translations update.
553. By Launchpad Translations on behalf of phablet-team on 2014-05-28: Launchpad automatic translations update.
554. By Launchpad Translations on behalf of phablet-team on 2014-05-29: Launchpad automatic translations update.
555. By Launchpad Translations on behalf of phablet-team on 2014-05-31: Launchpad automatic translations update.

Revision history for this message

Olivier Tilloy (osomon) wrote on 2014-06-02:

#

There’s a bunch of trivial conflicts when merging this branch into the latest trunk, can you please fix them?

Reply

Revision history for this message

Alexandre Abreu (abreu-alexandre) wrote on 2014-06-02:

#

done

Reply

Revision history for this message

Olivier Tilloy (osomon) wrote on 2014-06-02:

#

I have a bunch of comments, see inline.

Reply

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2014-06-02:

#

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

review: Needs Fixing (continuous-integration)

Reply

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2014-06-02:

#

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

review: Needs Fixing (continuous-integration)

Reply

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2014-06-02:

#

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

review: Needs Fixing (continuous-integration)

Reply

Revision history for this message

Alexandre Abreu (abreu-alexandre) wrote on 2014-06-03:

#

updated

Reply

Revision history for this message

Olivier Tilloy (osomon) wrote on 2014-06-03:

#

5 more comments inline.

Reply

Revision history for this message

Alexandre Abreu (abreu-alexandre) wrote on 2014-06-03:

#

To comment on a few notes:

> The type of the parameter for all those three signals should be 'url', not 'string'.

As I said earlier: yes I agree, but I get dbus runtime error on the AP side while trying to marshal the 'url' qml type to the QUrl qt type (unknown to dbus), ...

> this looks unsafe and abuse-prone to me, can’t we devise a more secure way of enabling tests, that doesn’t involve opening a backdoor in the container?

It is tricky to do something clean w/ this. Either way the container needs to run with some context, I did a few updates that makes it a bit cleaner ...

Reply

Revision history for this message

Olivier Tilloy (osomon) wrote on 2014-06-03:

#

> > The type of the parameter for all those three signals should be 'url', not
> 'string'.
>
> As I said earlier: yes I agree, but I get dbus runtime error on the AP side
> while trying to marshal the 'url' qml type to the QUrl qt type (unknown to
> dbus), ...

Ok, that makes sense.

> > this looks unsafe and abuse-prone to me, can’t we devise a more secure way
> of enabling tests, that doesn’t involve opening a backdoor in the container?
>
> It is tricky to do something clean w/ this. Either way the container needs to
> run with some context, I did a few updates that makes it a bit cleaner ...

I like the environment variable approach better. Can we not get rid of the --do-not-filter-url-patterns command line switch and rely only on the presence of the env var? (btw I would also check the value of the env var, if it’s "0" or "false" it should be ignored)

Reply

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2014-06-03:

#

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

review: Needs Fixing (continuous-integration)

Reply

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2014-06-03:

#

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

review: Needs Fixing (continuous-integration)

Reply

lp:~abreu-alexandre/webbrowser-app/add-ap-tests-for-navigation-and-popup updated on 2014-06-09

556. By Alberto Mardegan on 2014-06-02

Add online account support in the webapp container.

A new "accountProvider" command line parameter has been added to allow one to specify an account provider for the specific webapp launch. When used, one can specify a given provider, e.g. "facebook", to pull existing accounts (if any) from the specified provider
from Online Accounts.

When the provider does not have any existing account, an option to delegate the creation of such an account to OA is provided. It is also possible to skip the step and go straight to the target url.

557. By PS Jenkins bot on 2014-06-02

Releasing 0.23+14.10.20140602-0ubuntu1

558. By Alberto Mardegan on 2014-06-05

Fix the cookies unit tests

Make sure that the dbPath always refers to a file, not a directory.
Make sure that even on fast machines (or machines with a low-accuracy timer) the record timestamps are unique.

559. By Olivier Tilloy on 2014-06-05

Add a UA override rule for m.youtube.com to allow playing videos. Fixes: 1228415

560. By Olivier Tilloy on 2014-06-05

Do not display an empty contextual menu. Fixes: 1326752

561. By Michael Sheldon on 2014-06-05

Filter file uploads based on mime-type information (where available)

562. By PS Jenkins bot on 2014-06-05

Releasing 0.23+14.10.20140605-0ubuntu1

563. By Launchpad Translations on behalf of phablet-team on 2014-06-06

Launchpad automatic translations update.

564. By Launchpad Translations on behalf of phablet-team on 2014-06-08

Launchpad automatic translations update.

565. By Olivier Tilloy on 2014-06-09

Move version 0.2 of the API to "Ubuntu.Web" namespace, rename "UbuntuWebView" to "WebView", and document the public API intended for application developers.

The legacy namespace and component name are kept around for compatibility with existing applications.

Version 0.1 of the API remains untouched (it is deprecated).

Packaging changes:
- new qtdeclarative5-ubuntu-web-plugin package that contains the new namespace and name for the public QML API
- updated runtime dependencies for webbrowser-app, webapp-container and qtdeclarative5-ubuntu-ui-extras-browser-plugin
- updated descriptions for all packages
- added missing multiarch stanza for webapp-container-autopilot
- added missing predepends stanzas for multiarch packages Fixes: 1324180

566. By PS Jenkins bot on 2014-06-09

Releasing 0.23+14.10.20140609-0ubuntu1

Revision history for this message

Alexandre Abreu (abreu-alexandre) wrote on 2014-06-09:

#

updated

Reply

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2014-06-09:

#

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

review: Needs Fixing (continuous-integration)

Reply

lp:~abreu-alexandre/webbrowser-app/add-ap-tests-for-navigation-and-popup updated on 2014-07-03

567. By Olivier Tilloy on 2014-06-09

Use encodeURIComponent() to encode search queries entered in the address bar. Fixes: 1314673

568. By PS Jenkins bot on 2014-06-09

Releasing 0.23+14.10.20140609.1-0ubuntu1

569. By David Barth on 2014-06-12

Accept navigation inside popups if the redirection stays within the set of accepted URLs. This helps support the account switching feature in Gmail for example.

An additional heuristics ensures that trampoline URLs don't leave a blank window in their originating webapp, as we navigate back to the page just before the trampoline. This avoids regressions in Facebook, Twitter and other apps using popup redirects to open external links. Fixes: 1324848

570. By PS Jenkins bot on 2014-06-12

Releasing 0.23+14.10.20140612.1-0ubuntu1

571. By Launchpad Translations on behalf of phablet-team on 2014-06-17

Launchpad automatic translations update.

572. By Olivier Tilloy on 2014-06-17

debian/control: add fonts-liberation as a runtime dependency of webbrowser-app and webapp-container, for smoother rendering of webpages on devices. Fixes: 1322456

573. By Ugo Riboni on 2014-06-17

Update the application icon to the new suru theme.

This applies to desktop only. The current mobile theme (ubuntu-mobile-icons) still ships the old icon. Fixes: 1328147

574. By PS Jenkins bot on 2014-06-17

Releasing 0.23+14.10.20140617-0ubuntu1

575. By Launchpad Translations on behalf of phablet-team on 2014-06-18

Launchpad automatic translations update.

576. By Olivier Tilloy on 2014-06-18

Honour fullscreen requests. Fixes: 1308947, 1328168

577. By PS Jenkins bot on 2014-06-18

Releasing 0.23+14.10.20140618.2-0ubuntu1

578. By PS Jenkins bot on 2014-06-18

No change rebuild against Qt 5.3

579. By PS Jenkins bot on 2014-06-18

Releasing 0.23+14.10.20140618.3-0ubuntu1

580. By Olivier Tilloy on 2014-06-20

Re-enable the geolocation permission request dialog, now that it is implemented in oxide.
Fix it in the webkit-based webapp container. Fixes: 1182658

581. By Olivier Tilloy on 2014-06-20

Temporarily work around bug #1328839 in qtubuntu by not toggling fullscreen on the window on devices.

Once the Qt compositor work lands (scheduled for mid July), the bug should be resolved and this workaround can be removed. Fixes: 1328839

582. By PS Jenkins bot on 2014-06-20

Releasing 0.23+14.10.20140620-0ubuntu1

583. By Launchpad Translations on behalf of phablet-team on 2014-06-21

Launchpad automatic translations update.

584. By Launchpad Translations on behalf of phablet-team on 2014-06-24

Launchpad automatic translations update.

585. By Michael Sheldon on 2014-06-27

Add support for sharing links via content-hub. Fixes: 1294764

586. By PS Jenkins bot on 2014-06-27

Releasing 0.23+14.10.20140627-0ubuntu1

587. By Launchpad Translations on behalf of phablet-team on 2014-06-28

Launchpad automatic translations update.

588. By Launchpad Translations on behalf of phablet-team on 2014-06-29

Launchpad automatic translations update.

589. By Launchpad Translations on behalf of phablet-team on 2014-06-30

Launchpad automatic translations update.

590. By Olivier Tilloy on 2014-06-30

Re-enable contextual selection that had been disabled when switching to oxide.

Packaging change: renamed the qtdeclarative5-ubuntu-ui-extras-browser-plugin-assets package to qtdeclarative5-ubuntu-web-plugin-assets. Fixes: 1324292

591. By PS Jenkins bot on 2014-06-30

Releasing 0.23+14.10.20140630-0ubuntu1

592. By Olivier Tilloy on 2014-06-30

Add a user script that prevents so-called "smart banners" generated by the smartbanner jQuery plugin to show ads for native android/iOS apps. Fixes: 1329799

593. By Olivier Tilloy on 2014-06-30

Add support for custom search engines defined by the OpenSearch description document format
(http://www.opensearch.org/Specifications/OpenSearch/1.1). Fixes: 1277637, 1334546

594. By PS Jenkins bot on 2014-06-30

Releasing 0.23+14.10.20140630.1-0ubuntu1

595. By PS Jenkins bot on 2014-07-01

Resync trunk

596. By Launchpad Translations on behalf of phablet-team on 2014-07-02

Launchpad automatic translations update.

597. By Launchpad Translations on behalf of phablet-team on 2014-07-03

Launchpad automatic translations update.

598. By Alexandre Abreu on 2014-07-03

merge lp:~abreu-alexandre/webbrowser-app/add-ap-tests-for-navigation-and-popup

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2014-07-03:

#

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

review: Needs Fixing (continuous-integration)

Reply

lp:~abreu-alexandre/webbrowser-app/add-ap-tests-for-navigation-and-popup updated on 2014-07-03

599. By Alexandre Abreu on 2014-07-03: fix

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2014-07-03:

#

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

review: Approve (continuous-integration)

Reply

Revision history for this message

Alexandre Abreu (abreu-alexandre) wrote on 2014-07-04:

#

updated

Reply

Revision history for this message

Olivier Tilloy (osomon) wrote on 2014-07-07:

#

In a previous comment, I was suggesting to remove the "--block-open-external-urls" command-line switch, but I’m seeing what I assume is a leftover in src/app/webcontainer/webapp-container.cpp.

Reply

Revision history for this message

Olivier Tilloy (osomon) wrote on 2014-07-07:

#

$ pep8 tests
tests/autopilot/webapp_container/tests/__init__.py:56:80: E501 line too long (92 > 79 characters)
tests/autopilot/webapp_container/tests/__init__.py:75:80: E501 line too long (82 > 79 characters)

Can you please fix those two pep8 errors?

Reply

Revision history for this message

Alexandre Abreu (abreu-alexandre) wrote on 2014-07-09:

#

> In a previous comment, I was suggesting to remove the "--block-open-external-
> urls" command-line switch, but I’m seeing what I assume is a leftover in
> src/app/webcontainer/webapp-container.cpp.

I think your previous comment (that was addressed) was more about the "doFilterWebappUrlPatterns" flag which was now updated & cleaned w/ a env var approach,

I couldn't find references of a comment about the "--block" flag,

Reply

lp:~abreu-alexandre/webbrowser-app/add-ap-tests-for-navigation-and-popup updated on 2014-07-09

600. By Alexandre Abreu on 2014-07-09: pep8 fixes

Revision history for this message

Alexandre Abreu (abreu-alexandre) wrote on 2014-07-09:

#

> $ pep8 tests
> tests/autopilot/webapp_container/tests/__init__.py:56:80: E501 line too long
> (92 > 79 characters)
> tests/autopilot/webapp_container/tests/__init__.py:75:80: E501 line too long
> (82 > 79 characters)
>
>
> Can you please fix those two pep8 errors?

done

Reply

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2014-07-09:

#

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

review: Approve (continuous-integration)

Reply

Revision history for this message

Olivier Tilloy (osomon) wrote on 2014-07-10:

#

425 +from __future__ import absolute_import
522 +from __future__ import absolute_import

Those should probably be removed.

Reply

Revision history for this message

Olivier Tilloy (osomon) wrote on 2014-07-10:

#

445 + self.assertThat(lambda: webview.url,
446 + Eventually(Contains("/href-link")))

In case like this where comparing the value of a property on an object, the use of a lambda is superfluous, i.e. this will work:

self.assertThat(webview.url, Eventually(Contains("/href-link")))

There are a number of places in the new tests that can be simplified.

Reply

Revision history for this message

Olivier Tilloy (osomon) wrote on 2014-07-10:

#

To get back (again, sorry…) on the WEBAPP_CONTAINER_AP_TESTS_NOT_FILTER_URL_PATTERNS environment variable mechanism, if I understand correctly you added this "backdoor" because UrlPatternUtils::transformWebappSearchPatternToSafePattern(…) doesn’t recognize patterns that contain a port number, right?

If this is correct, how hard (and desirable) would it be to update the method to recognize patterns that contain a port number? It sounds like this could be useful, as the port number is part of the URL specification, so there could be webapps with a well-known domain name that require it, e.g. https://mycoolwebapp.com:12345/.

Reply

Revision history for this message

Olivier Tilloy (osomon) wrote on 2014-07-10:

#

545 + self.assertThat(lambda: popup_watcher.was_emitted,
546 + Eventually(Equals(popup_should_be_opened)))

In the case where popup_should_be_opened is False (i.e. on devices), I don’t think this test is useful, because popup_watcher.was_emitted is initially False anyway, before it changes to True.
So you probably want to make this assertion conditional instead:

if popup_should_be_opened:
self.assertThat(popup_watcher.was_emitted, Eventually(Equals(True)))

Reply

Revision history for this message

Olivier Tilloy (osomon) wrote on 2014-07-10:

#

547 + self.assertThat(lambda: url_watcher.was_emitted,
548 + Eventually(Equals(False)))

The same remark applies to this kind of assertion: url_watcher.was_emitted is initially False, and the use of Eventually() doesn’t mean that autopilot will wait for some time before initially testing the assertion. So you might as well change it to:

self.assertThat(url_watcher.was_emitted, Equals(False))

But this is racy at best. If you move it under the condition as suggested in the comment above, then it makes sense, because you check it after verifying that popup_watcher.was_emitted is True:

    if popup_should_be_opened:
        self.assertThat(popup_watcher.was_emitted, Eventually(Equals(True)))
        self.assertThat(url_watcher.was_emitted, Equals(False))

Reply

Revision history for this message

system-apps-ci-bot (system-apps-ci-bot) wrote on 2016-05-10:

#

FAILED: Continuous integration, rev:600
https://jenkins.canonical.com/system-apps/job/lp-webbrowser-app-ci/487/
Executed test runs:
FAILURE: https://jenkins.canonical.com/system-apps/job/build/489/console
FAILURE: https://jenkins.canonical.com/system-apps/job/build-0-fetch/489/console

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

review: Needs Fixing (continuous-integration)

Reply

webbrowser-app

Merge lp:~abreu-alexandre/webbrowser-app/add-ap-tests-for-navigation-and-popup into lp:webbrowser-app

Commit message

Description of the change

Unmerged revisions

Preview Diff

Subscribers

 === modified file 'src/app/webcontainer/WebApp.qml'
 --- src/app/webcontainer/WebApp.qml	2014-06-09 19:47:56 +0000
 +++ src/app/webcontainer/WebApp.qml	2014-07-09 13:56:27 +0000
@@ -38,6 +38,7 @@
      property alias webappName: webview.webappName
      property alias webappUrlPatterns: webview.webappUrlPatterns
      property alias popupRedirectionUrlPrefix: webview.popupRedirectionUrlPrefix
++    property alias blockOpenExternalUrls: webview.blockOpenExternalUrls
      actions: [
          Actions.Back {
@@ -68,6 +69,7 @@
          WebappContainerWebview {
              id: webview
++            objectName: "webview"
              toolbar: panel.panel
              anchors {
 === modified file 'src/app/webcontainer/WebViewImplOxide.qml'
 --- src/app/webcontainer/WebViewImplOxide.qml	2014-07-08 08:22:55 +0000
 +++ src/app/webcontainer/WebViewImplOxide.qml	2014-07-09 13:56:27 +0000
@@ -36,6 +36,16 @@
      property var webappUrlPatterns: null
      property string popupRedirectionUrlPrefix: ""
++    // Mostly used for testing & avoid external urls to
++    //  "leak" in the default browser
++    property bool blockOpenExternalUrls: false
++
++    // Those signals are used for testing purposes to externally
++    //  track down the various internal logic & steps of a popup lifecycle.
++    signal openExternalUrlTriggered(string url)
++    signal popupWindowOpened(string url)
++    signal popupWindowClosed(string url)
++
      currentWebview: webview
      contextualActions: ActionList {
@@ -63,7 +73,7 @@
+     }
      function haveValidUrlPatterns() {
--        return webappUrlPatterns && webappUrlPatterns.length !== 0
++        return webview.webappUrlPatterns && webview.webappUrlPatterns.length !== 0
+     }
      function isNewForegroundWebViewDisposition(disposition) {
@@ -95,13 +105,16 @@
          return false;
+     }
++    function openUrlExternally(url) {
++        webview.openExternalUrlTriggered(url)
++        if ( ! webview.blockOpenExternalUrls)
++            Qt.openUrlExternally(url)
++    }
++
      function navigationRequestedDelegate(request) {
          var newForegroundPageRequest = isNewForegroundWebViewDisposition(request.disposition)
          var url = request.url.toString()
--        console.log("\nwebview: " + webview.toString())
--        console.log("navigationRequestedDelegate - " + url)
--
          // Covers some edge cases corresponding to the default window.open() behavior.
          // When it is being called, the targetted URL will not load right away but
          // will first round trip to an "about:blank".
@@ -123,6 +136,11 @@
                      : url;
              if (webview.shouldAllowNavigationTo(targetUrl)) {
++                if (formFactor === "desktop") {
++                    request.action = Oxide.NavigationRequest.ActionAccept
++                    return
++                }
++
                  console.debug('Redirecting popup browsing ' + targetUrl + ' in the current container window.')
                  request.action = Oxide.NavigationRequest.ActionReject
                  webappContainerHelper.browseToUrlRequested(webview, url.slice(url.indexOf(popupRedirectionUrlPrefix)))
@@ -132,7 +150,7 @@
              if (shouldOpenPopupsInDefaultBrowser()) {
                  console.debug('Opening popup window ' + url + ' in the browser window.')
                  request.action = Oxide.NavigationRequest.ActionReject
--                Qt.openUrlExternally(url)
++                openUrlExternally(url)
                  return;
+             }
              return
@@ -169,7 +187,7 @@
          if (request.action === Oxide.NavigationRequest.ActionReject) {
              console.debug('Opening: ' + url + ' in the browser window.')
--            Qt.openUrlExternally(url)
++            openUrlExternally(url)
+         }
+     }
@@ -193,7 +211,7 @@
                      if ( ! isNewForegroundWebViewDisposition(request.disposition) &&
                              ! webview.shouldAllowNavigationTo(url)) {
                          request.action = Oxide.NavigationRequest.ActionReject
--                        Qt.openUrlExternally(url);
++                        openUrlExternally(url)
                          popup.close()
                          return;
+                     }
@@ -223,12 +241,20 @@
                          return;
                      if (_url != 'about:blank' && ! webview.shouldAllowNavigationTo(_url)) {
--                        Qt.openUrlExternally(_url);
++                        openUrlExternally(url)
                          popup.close()
+                     }
+                 }
+             }
--            Component.onCompleted: popup.show()
++
++            onVisibleChanged: {
++                if (popup.visible)
++                    popupWindowOpened(popupBrowser.url);
++                else
++                    popupWindowClosed(popupBrowser.url);
++            }
++
++            Component.onCompleted: show()
+         }
+     }
 === modified file 'src/app/webcontainer/WebViewImplWebkit.qml'
 --- src/app/webcontainer/WebViewImplWebkit.qml	2014-06-18 05:55:37 +0000
 +++ src/app/webcontainer/WebViewImplWebkit.qml	2014-07-09 13:56:27 +0000
@@ -41,6 +41,9 @@
          return webview.localUserAgentOverride.length === 0 ? undefined : webview.localUserAgentOverride
+     }
++    // Is a no-op for QtWebkit
++    property bool blockOpenExternalUrls: false
++
      experimental.certificateVerificationDialog: CertificateVerificationDialog {}
      experimental.authenticationDialog: AuthenticationDialog {}
      experimental.proxyAuthenticationDialog: ProxyAuthenticationDialog {}
 === modified file 'src/app/webcontainer/WebappContainerWebview.qml'
 --- src/app/webcontainer/WebappContainerWebview.qml	2014-06-09 19:47:56 +0000
 +++ src/app/webcontainer/WebappContainerWebview.qml	2014-07-09 13:56:27 +0000
@@ -32,12 +32,14 @@
      property string webappName: ""
      property var currentWebview: webappContainerWebViewLoader.item
      property var toolbar: null
++    property bool blockOpenExternalUrls: false
      property var webappUrlPatterns
      property string localUserAgentOverride: ""
      property string popupRedirectionUrlPrefix: ""
      Loader {
          id: webappContainerWebViewLoader
++        objectName: "containerWebviewLoader"
          anchors.fill: parent
+     }
@@ -55,7 +57,8 @@
                      , webappName: containerWebview.webappName
                      , webappUrlPatterns: containerWebview.webappUrlPatterns
                      , developerExtrasEnabled: containerWebview.developerExtrasEnabled
--                    , popupRedirectionUrlPrefix: containerWebview.popupRedirectionUrlPrefix})
++                    , popupRedirectionUrlPrefix: containerWebview.popupRedirectionUrlPrefix
++                    , blockOpenExternalUrls: containerWebview.blockOpenExternalUrls})
+     }
+ }
 === modified file 'src/app/webcontainer/webapp-container.cpp'
 --- src/app/webcontainer/webapp-container.cpp	2014-07-02 14:42:06 +0000
 +++ src/app/webcontainer/webapp-container.cpp	2014-07-09 13:56:27 +0000
@@ -88,7 +88,8 @@
      m_backForwardButtonsVisible(false),
      m_addressBarVisible(false),
      m_localWebappManifest(false),
--    m_webappContainerHelper(new WebappContainerHelper())
++    m_webappContainerHelper(new WebappContainerHelper()),
++    m_blockOpenExternalUrls(false)
+ {
+ }
@@ -118,6 +119,8 @@
          qDebug() << "Using" << (m_withOxide ? "Oxide" : "QtWebkit") << "as the web engine backend";
          m_window->setProperty("oxide", m_withOxide);
++        m_window->setProperty("blockOpenExternalUrls", m_blockOpenExternalUrls);
++
          m_window->setProperty("webappUrlPatterns", m_webappUrlPatterns);
          QQmlContext* context = m_engine->rootContext();
          if (m_storeSessionCookies) {
@@ -201,6 +204,20 @@
  void WebappContainer::parseCommandLine()
+ {
++    const QString AP_TESTS_NOT_FILTER_URL_PATTERNS_ENV_VAR =
++            qgetenv("WEBAPP_CONTAINER_AP_TESTS_NOT_FILTER_URL_PATTERNS");
++
++    // Used internally for testing when the browsed urls
++    // are not totally properly formed for strict url patterns
++    // (localhost:port, <ip>:port, etc.)
++    bool doFilterWebappUrlPatterns =
++            (AP_TESTS_NOT_FILTER_URL_PATTERNS_ENV_VAR != "1");
++
++    if ( ! doFilterWebappUrlPatterns) {
++        qWarning() << "NOT filtering webapp url patterns. "
++                      "Only use this for internal testing, not production.";
++    }
++
      Q_FOREACH(const QString& argument, m_arguments) {
          if (argument == "--webkit") {
              // force webkit
@@ -219,7 +236,10 @@
              QString tail = argument.split("--webappUrlPatterns=")[1];
              if (!tail.isEmpty()) {
                  QStringList includePatterns = tail.split(URL_PATTERN_SEPARATOR);
--                m_webappUrlPatterns = UrlPatternUtils::filterAndTransformUrlPatterns(includePatterns);
++                m_webappUrlPatterns =
++                    doFilterWebappUrlPatterns ?
++                          UrlPatternUtils::filterAndTransformUrlPatterns(includePatterns)
++                        : includePatterns;
+             }
          } else if (argument.startsWith("--accountProvider=")) {
              m_accountProvider = argument.split("--accountProvider=")[1];
@@ -233,6 +253,8 @@
              m_localWebappManifest = true;
          } else if (argument.startsWith("--popup-redirection-url-prefix=")) {
              m_popupRedirectionUrlPrefix = argument.split("--popup-redirection-url-prefix=")[1];;
++        } else if (argument == "--block-open-external-urls") {
++            m_blockOpenExternalUrls = true;
+         }
+     }
+ }
 === modified file 'src/app/webcontainer/webapp-container.h'
 --- src/app/webcontainer/webapp-container.h	2014-06-09 19:47:56 +0000
 +++ src/app/webcontainer/webapp-container.h	2014-07-09 13:56:27 +0000
@@ -58,6 +58,7 @@
      bool m_localWebappManifest;
      QString m_popupRedirectionUrlPrefix;
      QScopedPointer<WebappContainerHelper> m_webappContainerHelper;
++    bool m_blockOpenExternalUrls;
      static const QString URL_PATTERN_SEPARATOR;
  };
 === modified file 'src/app/webcontainer/webapp-container.qml'
 --- src/app/webcontainer/webapp-container.qml	2014-07-02 13:01:14 +0000
 +++ src/app/webcontainer/webapp-container.qml	2014-07-09 13:56:27 +0000
@@ -39,6 +39,7 @@
      property string accountProvider: ""
      property string popupRedirectionUrlPrefix: ""
      property var __webappCookieStore: null
++    property bool blockOpenExternalUrls: false
      contentOrientation: Screen.orientation
@@ -75,6 +76,7 @@
              webappModelSearchPath: root.webappModelSearchPath
              webappName: root.webappName
              webappUrlPatterns: root.webappUrlPatterns
++            blockOpenExternalUrls: root.blockOpenExternalUrls
              popupRedirectionUrlPrefix: root.popupRedirectionUrlPrefix
 === modified file 'tests/autopilot/webapp_container/tests/__init__.py'
 --- tests/autopilot/webapp_container/tests/__init__.py	2014-05-02 05:44:36 +0000
 +++ tests/autopilot/webapp_container/tests/__init__.py	2014-07-09 13:56:27 +0000
@@ -43,12 +43,18 @@
              return LOCAL_BROWSER_CONTAINER_PATH_NAME
          return INSTALLED_BROWSER_CONTAINER_PATH_NAME
++    def setUp(self):
++        self.pointing_device = toolkit_emulators.get_pointing_device()
++        super(WebappContainerTestCaseBase, self).setUp()
++
      def launch_webcontainer_app(self, args):
          if model() != 'Desktop':
              args.append(
                  '--desktop_file_hint=/usr/share/applications/'
                  'webbrowser-app.desktop')
          try:
++            self.patch_environment(
++                "WEBAPP_CONTAINER_AP_TESTS_NOT_FILTER_URL_PATTERNS", "1")
              self.app = self.launch_test_application(
                  self.get_webcontainer_app_path(),
                  *args,
@@ -62,6 +68,14 @@
      def get_webcontainer_window(self):
          return self.app.select_single(objectName="webappContainer")
++    def get_webview(self):
++        return self.app.select_single(objectName="webview")
++
++    def get_oxide_webview(self):
++        container = self.get_webview().select_single(
++            objectName='containerWebviewLoader')
++        return container.select_single('WebViewImplOxide')
++
      def get_webcontainer_webview(self):
          return self.app.select_single(objectName="webappBrowserView")
 === modified file 'tests/autopilot/webapp_container/tests/fake_servers.py'
 --- tests/autopilot/webapp_container/tests/fake_servers.py	2014-04-24 10:50:42 +0000
 +++ tests/autopilot/webapp_container/tests/fake_servers.py	2014-07-09 13:56:27 +0000
@@ -42,6 +42,90 @@
          if self.path == '/':
              self.send_response(200)
              self.serve_content(self.basic_html_content())
++        elif self.path == '/href-link':
++            self.send_response(200)
++            html_response = """
++            <html>
++            <head>
++            <script>
++            window.onload = function() {
++              document.addEventListener('click', function() {
++                document.querySelector('div a').click();
++              });
++            };
++            </script>
++            </head>
++            <body>
++            <div style="width: 100%; height: 100%">
++                <h1><a href="target-blank">local href link</a></h1>
++            </div>
++            </body>
++            </html>
++            """
++            self.serve_content(html_response)
++        elif self.path == '/redirect':
++            self.send_response(303)
++            self.send_header("Location", "href-link")
++            self.end_headers()
++        elif self.path == '/window-open-to-redirect':
++            self.send_response(200)
++            html_response = """
++            <html>
++            <head>
++            <script>
++            window.onload = function() {
++              document.addEventListener('click', function() {
++                window.open("redirect");
++              });
++            };
++            </script>
++            </head>
++            <body>
++            <div style="width: 100%; height: 100%"></div>
++            </body>
++            </html>
++            """
++            self.serve_content(html_response)
++        elif self.path == '/window-open':
++            self.send_response(200)
++            html_response = """
++            <html>
++            <head>
++            <script>
++            window.onload = function() {
++              document.addEventListener('click', function() {
++                window.open("window-open");
++              });
++            };
++            </script>
++            </head>
++            <body>
++            <div style="width: 100%; height: 100%"></div>
++            </body>
++            </html>
++            """
++            self.serve_content(html_response)
++        elif self.path == '/target-blank':
++            self.send_response(200)
++            html_response = """
++            <html>
++            <head>
++            <script>
++            window.onload = function() {
++              document.addEventListener('click', function() {
++                document.querySelector('div a').click();
++              });
++            };
++            </script>
++            </head>
++            <body>
++            <div style="width: 100%; height: 100%">
++            <h1><a href="target-blank" target="_blank">target-blank</a></h1>
++            </div>
++            </body>
++            </html>
++            """
++            self.serve_content(html_response)
          else:
              self.send_error(404)
 === added file 'tests/autopilot/webapp_container/tests/test_navigation_filtering.py'
 --- tests/autopilot/webapp_container/tests/test_navigation_filtering.py	1970-01-01 00:00:00 +0000
 +++ tests/autopilot/webapp_container/tests/test_navigation_filtering.py	2014-07-09 13:56:27 +0000
@@ -0,0 +1,92 @@
++# -*- Mode: Python; coding: utf-8; indent-tabs-mode: nil; tab-width: 4 -*-
++# Copyright 2014 Canonical
++#
++# This program is free software: you can redistribute it and/or modify it
++# under the terms of the GNU General Public License version 3, as published
++# by the Free Software Foundation.
++#
++# This program is distributed in the hope that it will be useful,
++# but WITHOUT ANY WARRANTY; without even the implied warranty of
++# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
++# GNU General Public License for more details.
++#
++# You should have received a copy of the GNU General Public License
++# along with this program. If not, see <http://www.gnu.org/licenses/>.
++
++from __future__ import absolute_import
++
++from testtools import skipUnless
++from testtools.matchers import Equals, Contains
++from autopilot.matchers import Eventually
++from autopilot import platform
++
++from webapp_container.tests import WebappContainerTestCaseWithLocalContentBase
++
++
++class WebappContainerNavivationFilteringTestCase(
++        WebappContainerTestCaseWithLocalContentBase):
++
++    def test_no_patterns(self):
++        args = []
++        self.launch_webcontainer_app_with_local_http_server(args, "/href-link")
++
++        webview = self.get_oxide_webview()
++        url_watcher = webview.watch_signal('urlChanged()')
++
++        self.assertThat(lambda: webview.url,
++                        Eventually(Contains("/href-link")))
++
++        self.pointing_device.click_object(webview)
++        self.assertThat(lambda: url_watcher.was_emitted,
++                        Eventually(Equals(True)))
++        self.assertThat(lambda: webview.url,
++                        Eventually(Contains("/target-blank")))
++
++    def test_url_pattern_filtering(self):
++        args = ['--webappUrlPatterns=' + self.base_url + '/href-link',
++                '--block-open-external-urls']
++        self.launch_webcontainer_app_with_local_http_server(args, "/href-link")
++
++        webview = self.get_oxide_webview()
++        self.assertThat(lambda: webview.url,
++                        Eventually(Contains("/href-link")))
++
++        external_open_watcher = webview.watch_signal(
++            'openExternalUrlTriggered(QString)')
++        url_watcher = webview.watch_signal('urlChanged()')
++
++        self.pointing_device.click_object(webview)
++        self.assertThat(lambda: external_open_watcher.was_emitted,
++                        Eventually(Equals(True)))
++        self.assertThat(url_watcher.was_emitted,
++                        Equals(False))
++
++    @skipUnless(platform.model() == 'Desktop', "Only runs on the Desktop")
++    def test_url_pattern_filtering_in_popup(self):
++        args = ['--webappUrlPatterns=' + self.base_url
++                + '/window-open-to-redirect,' + self.base_url + '/redirect',
++                '--block-open-external-urls']
++        self.launch_webcontainer_app_with_local_http_server(
++            args, "/window-open-to-redirect")
++
++        webview = self.get_oxide_webview()
++        self.assertThat(lambda: webview.url,
++                        Eventually(Contains("/window-open-to-redirect")))
++
++        external_open_watcher = webview.watch_signal(
++            'openExternalUrlTriggered(QString)')
++        url_watcher = webview.watch_signal('urlChanged()')
++        popup_watcher = webview.watch_signal('popupWindowOpened(QString)')
++        popup_closed_watcher = webview.watch_signal(
++            'popupWindowClosed(QString)')
++
++        self.pointing_device.click_object(webview)
++
++        self.assertThat(lambda: popup_watcher.was_emitted,
++                        Eventually(Equals(True)))
++        self.assertThat(lambda: external_open_watcher.was_emitted,
++                        Eventually(Equals(True)))
++        self.assertThat(lambda: popup_closed_watcher.was_emitted,
++                        Eventually(Equals(True)))
++        self.assertThat(url_watcher.was_emitted,
++                        Equals(False))
 === added file 'tests/autopilot/webapp_container/tests/test_navigation_popup.py'
 --- tests/autopilot/webapp_container/tests/test_navigation_popup.py	1970-01-01 00:00:00 +0000
 +++ tests/autopilot/webapp_container/tests/test_navigation_popup.py	2014-07-09 13:56:27 +0000
@@ -0,0 +1,58 @@
++# -*- Mode: Python; coding: utf-8; indent-tabs-mode: nil; tab-width: 4 -*-
++# Copyright 2014 Canonical
++#
++# This program is free software: you can redistribute it and/or modify it
++# under the terms of the GNU General Public License version 3, as published
++# by the Free Software Foundation.
++#
++# This program is distributed in the hope that it will be useful,
++# but WITHOUT ANY WARRANTY; without even the implied warranty of
++# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
++# GNU General Public License for more details.
++#
++# You should have received a copy of the GNU General Public License
++# along with this program. If not, see <http://www.gnu.org/licenses/>.
++
++from __future__ import absolute_import
++
++from testtools.matchers import Equals
++from autopilot.matchers import Eventually
++from autopilot import platform
++
++from webapp_container.tests import WebappContainerTestCaseWithLocalContentBase
++
++
++class WebappContainerNavivationRequestPolicyTestCase(
++        WebappContainerTestCaseWithLocalContentBase):
++
++    def test_popup_window_open(self):
++        args = ['--block-open-external-urls']
++        popup_should_be_opened = platform.model() == "Desktop"
++        self.launch_webcontainer_app_with_local_http_server(
++            args, "/window-open")
++
++        webview = self.get_oxide_webview()
++        popup_watcher = webview.watch_signal('popupWindowOpened(QString)')
++        url_watcher = webview.watch_signal('urlChanged()')
++
++        self.pointing_device.click_object(webview)
++        self.assertThat(lambda: popup_watcher.was_emitted,
++                        Eventually(Equals(popup_should_be_opened)))
++        self.assertThat(lambda: url_watcher.was_emitted,
++                        Eventually(Equals(False)))
++
++    def test_popup_target_blank(self):
++        args = ['--block-open-external-urls']
++        popup_should_be_opened = platform.model() == "Desktop"
++        self.launch_webcontainer_app_with_local_http_server(
++            args, "/target-blank")
++
++        webview = self.get_oxide_webview()
++        popup_watcher = webview.watch_signal('popupWindowOpened(QString)')
++        url_watcher = webview.watch_signal('urlChanged()')
++
++        self.pointing_device.click_object(webview)
++        self.assertThat(lambda: popup_watcher.was_emitted,
++                        Eventually(Equals(popup_should_be_opened)))
++        self.assertThat(lambda: url_watcher.was_emitted,
++                        Eventually(Equals(False)))