Merge lp:~abreu-alexandre/webbrowser-app/webapp-navigation-handling into lp:webbrowser-app

[Alexandre Abreu]

Navigation support

Navigation support

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:291
http://jenkins.qa.ubuntu.com/job/webbrowser-app-ci/293/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-saucy/2936/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-touch/503/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/webbrowser-app-saucy-amd64-ci/176
    SUCCESS: http://jenkins.qa.ubuntu.com/job/webbrowser-app-saucy-armhf-ci/176
        deb: http://jenkins.qa.ubuntu.com/job/webbrowser-app-saucy-armhf-ci/176/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/webbrowser-app-saucy-i386-ci/176
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-saucy-i386/2942
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-saucy-i386/2942/artifact/work/output/*zip*/output.zip
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-runner-saucy/2499/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-saucy-armhf/505
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-saucy-armhf/505/artifact/work/output/*zip*/output.zip
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-runner-maguro/451/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-runner-mako/459/console

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

review: Needs Fixing (continuous-integration)

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:292
http://jenkins.qa.ubuntu.com/job/webbrowser-app-ci/294/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-saucy/2946/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-touch/513/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/webbrowser-app-saucy-amd64-ci/177
    SUCCESS: http://jenkins.qa.ubuntu.com/job/webbrowser-app-saucy-armhf-ci/177
        deb: http://jenkins.qa.ubuntu.com/job/webbrowser-app-saucy-armhf-ci/177/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/webbrowser-app-saucy-i386-ci/177
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-saucy-i386/2952
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-saucy-i386/2952/artifact/work/output/*zip*/output.zip
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-runner-saucy/2508/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-saucy-armhf/515
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-saucy-armhf/515/artifact/work/output/*zip*/output.zip
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-runner-maguro/460/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-runner-mako/468/console

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

review: Needs Fixing (continuous-integration)

[Olivier Tilloy (osomon)]

17 + property var onNavigationRequestedDelegate: null
18 UserAgent {
19 id: userAgent
20 }
21 experimental.userAgent: userAgent.defaultUA
22 onNavigationRequested: {
23 + if (onNavigationRequestedDelegate && typeof(onNavigationRequestedDelegate) == 'function') {
24 + onNavigationRequestedDelegate (request);
25 + if (request.action === WebView.IgnoreRequest)
26 + return;
27 + }

This works, however I’m not convinced by the way it is implemented.
How about, instead of a property for a delegate function, implemented a default method in the base class (UbuntuWebView), called e.g. handleNavigationRequest(…), have onNavigationRequested call it, and override it in Browser.qml?

This would be functionally equivalent, but more elegant in my opinion.

[Olivier Tilloy (osomon)]

8 -import QtWebKit 3.0
9 +import QtWebKit 3.1

This will require the new version of QtWebKit to be in the archive, and backported to raring in a PPA. Is it already there?

review: Needs Information

[Olivier Tilloy (osomon)]

73 + var url = request.url + ' ';

Why is this needed?

[Olivier Tilloy (osomon)]

84 + var pattern = browser.urlIncludesPatterns[i].replace(/\*/g, '[^ ]*');

Why do you need to replace wildcards by '[^ ]*' ?
Couldn’t this substitution be done earlier on, when browser.urlIncludesPatterns is set (when the app is launched)?

[Olivier Tilloy (osomon)]

82 + var matched = false;

Instead of using an intermediate boolean var, how about initially setting action to WebView.IgnoreRequest just before the for loop, and if a given include pattern matches, set it to WebView.AcceptRequest ?

[Olivier Tilloy (osomon)]

226 + property alias includes: browser.urlIncludesPatterns

Can this property be renamed to something more explicit? I’d like to see 'webapps' in the name, to make it crystal clear that this property affects only webapps.

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:292
http://jenkins.qa.ubuntu.com/job/webbrowser-app-ci/297/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-saucy/3036/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-touch/597/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/webbrowser-app-saucy-amd64-ci/180
    SUCCESS: http://jenkins.qa.ubuntu.com/job/webbrowser-app-saucy-armhf-ci/180
        deb: http://jenkins.qa.ubuntu.com/job/webbrowser-app-saucy-armhf-ci/180/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/webbrowser-app-saucy-i386-ci/180
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-saucy-i386/3042
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-saucy-i386/3042/artifact/work/output/*zip*/output.zip
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-runner-saucy/2570/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-saucy-armhf/599
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-saucy-armhf/599/artifact/work/output/*zip*/output.zip
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-runner-maguro/521/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-runner-mako/533/console

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

review: Needs Fixing (continuous-integration)

[Alexandre Abreu (abreu-alexandre)]

> 17 + property var onNavigationRequestedDelegate: null
> 18 UserAgent {
> 19 id: userAgent
> 20 }
> 21 experimental.userAgent: userAgent.defaultUA
> 22 onNavigationRequested: {
> 23 + if (onNavigationRequestedDelegate &&
> typeof(onNavigationRequestedDelegate) == 'function') {
> 24 + onNavigationRequestedDelegate (request);
> 25 + if (request.action === WebView.IgnoreRequest)
> 26 + return;
> 27 + }
>
> This works, however I’m not convinced by the way it is implemented.
> How about, instead of a property for a delegate function, implemented a
> default method in the base class (UbuntuWebView), called e.g.
> handleNavigationRequest(…), have onNavigationRequested call it, and override
> it in Browser.qml?
>
> This would be functionally equivalent, but more elegant in my opinion.

I am not really sure about that one, since it gives too much liberty to the
UbuntuWebView's client/user. The semantics is supposed to be a delegation of
some specialized behaviour that could override the basic one; but w/ your solution
nothing prevents the client to forget to call the default behaviour, or for example
behaviours from colliding/conflicting in their side-effects,

[Alexandre Abreu (abreu-alexandre)]

> 73 + var url = request.url + ' ';
>
> Why is this needed?

because request.url is not a js string, but an object that evaluates to a string, I force the coercion this way,

[Alexandre Abreu (abreu-alexandre)]

> 8 -import QtWebKit 3.0
> 9 +import QtWebKit 3.1
>
> This will require the new version of QtWebKit to be in the archive, and
> backported to raring in a PPA. Is it already there?

yes it's in the archive for S, I'll validate that's it's there for R w/ Mirv
before top-approving,

[Alexandre Abreu (abreu-alexandre)]

> 226 + property alias includes: browser.urlIncludesPatterns
>
> Can this property be renamed to something more explicit? I’d like to see
> 'webapps' in the name, to make it crystal clear that this property affects
> only webapps.

done.

[Alexandre Abreu (abreu-alexandre)]

> 82 + var matched = false;
>
> Instead of using an intermediate boolean var, how about initially setting
> action to WebView.IgnoreRequest just before the for loop, and if a given
> include pattern matches, set it to WebView.AcceptRequest ?

good idea, done

[Alexandre Abreu (abreu-alexandre)]

> 84 + var pattern =
> browser.urlIncludesPatterns[i].replace(/\*/g, '[^ ]*');
>
> Why do you need to replace wildcards by '[^ ]*' ?
> Couldn’t this substitution be done earlier on, when
> browser.urlIncludesPatterns is set (when the app is launched)?

+100, done

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:293
http://jenkins.qa.ubuntu.com/job/webbrowser-app-ci/300/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-saucy/3068/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-touch/629/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/webbrowser-app-saucy-amd64-ci/183
    SUCCESS: http://jenkins.qa.ubuntu.com/job/webbrowser-app-saucy-armhf-ci/183
        deb: http://jenkins.qa.ubuntu.com/job/webbrowser-app-saucy-armhf-ci/183/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/webbrowser-app-saucy-i386-ci/183
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-saucy-i386/3074/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-saucy-armhf/631/console

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

review: Needs Fixing (continuous-integration)

[Olivier Tilloy (osomon)]

> > 17 + property var onNavigationRequestedDelegate: null
> > 18 UserAgent {
> > 19 id: userAgent
> > 20 }
> > 21 experimental.userAgent: userAgent.defaultUA
> > 22 onNavigationRequested: {
> > 23 + if (onNavigationRequestedDelegate &&
> > typeof(onNavigationRequestedDelegate) == 'function') {
> > 24 + onNavigationRequestedDelegate (request);
> > 25 + if (request.action === WebView.IgnoreRequest)
> > 26 + return;
> > 27 + }
> >
> > This works, however I’m not convinced by the way it is implemented.
> > How about, instead of a property for a delegate function, implemented a
> > default method in the base class (UbuntuWebView), called e.g.
> > handleNavigationRequest(…), have onNavigationRequested call it, and override
> > it in Browser.qml?
> >
> > This would be functionally equivalent, but more elegant in my opinion.
>
> I am not really sure about that one, since it gives too much liberty to the
> UbuntuWebView's client/user. The semantics is supposed to be a delegation of
> some specialized behaviour that could override the basic one; but w/ your
> solution
> nothing prevents the client to forget to call the default behaviour, or for
> example
> behaviours from colliding/conflicting in their side-effects,

I’m not sure I’m following you. What I’m proposing is a different implementation, but the semantics are exactly the same: the specialized behavior is delegated to a function (which by default does nothing and can be overridden) instead of being delegated to a function object stored as a property.

[Olivier Tilloy (osomon)]

> > 73 + var url = request.url + ' ';
> >
> > Why is this needed?
>
> because request.url is not a js string, but an object that evaluates to a
> string, I force the coercion this way,

Wouldn’t it be cleaner (and strictly equivalent) to do: var url = request.url.toString() ?

[Olivier Tilloy (osomon)]

> > 226 + property alias includes: browser.urlIncludesPatterns
> >
> > Can this property be renamed to something more explicit? I’d like to see
> > 'webapps' in the name, to make it crystal clear that this property affects
> > only webapps.
>
> done.

Thanks. Could you also rename the command-line argument and the property of the CommandLineParser class, for consistency?

[Alexandre Abreu (abreu-alexandre)]

>
> Thanks. Could you also rename the command-line argument and the property of
> the CommandLineParser class, for consistency?

done

[Alexandre Abreu (abreu-alexandre)]

>
> Wouldn’t it be cleaner (and strictly equivalent) to do: var url =
> request.url.toString() ?

done, although the previous expression is quite idiomatic

[Alexandre Abreu (abreu-alexandre)]

> > > 17 + property var onNavigationRequestedDelegate: null
> > > 18 UserAgent {
> > > 19 id: userAgent
> > > 20 }
> > > 21 experimental.userAgent: userAgent.defaultUA
> > > 22 onNavigationRequested: {
> > > 23 + if (onNavigationRequestedDelegate &&
> > > typeof(onNavigationRequestedDelegate) == 'function') {
> > > 24 + onNavigationRequestedDelegate (request);
> > > 25 + if (request.action === WebView.IgnoreRequest)
> > > 26 + return;
> > > 27 + }
> > >
> > > This works, however I’m not convinced by the way it is implemented.
> > > How about, instead of a property for a delegate function, implemented a
> > > default method in the base class (UbuntuWebView), called e.g.
> > > handleNavigationRequest(…), have onNavigationRequested call it, and
> override
> > > it in Browser.qml?
> > >
> > > This would be functionally equivalent, but more elegant in my opinion.
> >
> > I am not really sure about that one, since it gives too much liberty to the
> > UbuntuWebView's client/user. The semantics is supposed to be a delegation of
> > some specialized behaviour that could override the basic one; but w/ your
> > solution
> > nothing prevents the client to forget to call the default behaviour, or for
> > example
> > behaviours from colliding/conflicting in their side-effects,
>
> I’m not sure I’m following you. What I’m proposing is a different
> implementation, but the semantics are exactly the same: the specialized
> behavior is delegated to a function (which by default does nothing and can be
> overridden) instead of being delegated to a function object stored as a
> property.

forget about my comment, I read to quickly and overlooked the fact that the
property usage was the issue ..

done

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:294
http://jenkins.qa.ubuntu.com/job/webbrowser-app-ci/305/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-saucy/3089/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-touch/650/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/webbrowser-app-saucy-amd64-ci/188
    SUCCESS: http://jenkins.qa.ubuntu.com/job/webbrowser-app-saucy-armhf-ci/188
        deb: http://jenkins.qa.ubuntu.com/job/webbrowser-app-saucy-armhf-ci/188/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/webbrowser-app-saucy-i386-ci/188
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-saucy-i386/3095
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-saucy-i386/3095/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-saucy-armhf/652
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-saucy-armhf/652/artifact/work/output/*zip*/output.zip

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

review: Needs Fixing (continuous-integration)

[Olivier Tilloy (osomon)]

124 +#include <QtCore/QRegularExpression>

It looks like this is unused. Can it be removed?

[Alexandre Abreu (abreu-alexandre)]

> 124 +#include <QtCore/QRegularExpression>
>
> It looks like this is unused. Can it be removed?

done

[Olivier Tilloy (osomon)]

In the two implementations of navigationRequestedDelegate(…), can the calls to console.debug() be removed?
Also, could you please add a comment in the default implementation (the one that does nothing) to explain that items inheriting UbuntuWebView may override this method to implement a specific behavior?

[Alexandre Abreu (abreu-alexandre)]

> In the two implementations of navigationRequestedDelegate(…), can the calls to
> console.debug() be removed?
> Also, could you please add a comment in the default implementation (the one
> that does nothing) to explain that items inheriting UbuntuWebView may override
> this method to implement a specific behavior?

done

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:295
http://jenkins.qa.ubuntu.com/job/webbrowser-app-ci/307/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-saucy/3092/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-touch/652/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/webbrowser-app-saucy-amd64-ci/190
    SUCCESS: http://jenkins.qa.ubuntu.com/job/webbrowser-app-saucy-armhf-ci/190
        deb: http://jenkins.qa.ubuntu.com/job/webbrowser-app-saucy-armhf-ci/190/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/webbrowser-app-saucy-i386-ci/190
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-saucy-i386/3098
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-saucy-i386/3098/artifact/work/output/*zip*/output.zip
    ABORTED: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-runner-saucy/2591/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-saucy-armhf/654
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-saucy-armhf/654/artifact/work/output/*zip*/output.zip

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

review: Needs Fixing (continuous-integration)

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:296
http://jenkins.qa.ubuntu.com/job/webbrowser-app-ci/308/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-saucy/3095/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-touch/655/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/webbrowser-app-saucy-amd64-ci/191
    SUCCESS: http://jenkins.qa.ubuntu.com/job/webbrowser-app-saucy-armhf-ci/191
        deb: http://jenkins.qa.ubuntu.com/job/webbrowser-app-saucy-armhf-ci/191/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/webbrowser-app-saucy-i386-ci/191
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-saucy-i386/3101
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-saucy-i386/3101/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-runner-saucy/2594
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-saucy-armhf/657
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-saucy-armhf/657/artifact/work/output/*zip*/output.zip

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

review: Needs Fixing (continuous-integration)

[Olivier Tilloy (osomon)]

Looks good to me. Approving, and I’ll let you top-approve once we’re sure the required update to QtWebKit has been backported to Raring.

review: Approve

[Alexandre Abreu (abreu-alexandre)]

Timo just told me that the QtWebkit patch has been backported to R and build into the qtwebkit release found in the SDK ppa (version 5.0.1-0ubuntu7~raring1~test2)

[PS Jenkins bot (ps-jenkins)]

FAILED: Autolanding.
More details in the following jenkins job:
http://jenkins.qa.ubuntu.com/job/webbrowser-app-autolanding/176/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-saucy/3406/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-touch/935/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/webbrowser-app-saucy-amd64-autolanding/120/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/webbrowser-app-saucy-armhf-autolanding/120/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/webbrowser-app-saucy-i386-autolanding/120/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-saucy-i386/3417/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-saucy-armhf/937/console

review: Needs Fixing (continuous-integration)

[Olivier Tilloy (osomon)]

It appears there are conflicts when merging into the latest trunk.

[PS Jenkins bot (ps-jenkins)]

review: Approve (continuous-integration)

[PS Jenkins bot (ps-jenkins)]

FAILED: Autolanding.
Unapproved changes made after approval.
http://jenkins.qa.ubuntu.com/job/webbrowser-app-autolanding/183/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-saucy/3421
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-touch/949
    SUCCESS: http://jenkins.qa.ubuntu.com/job/webbrowser-app-saucy-amd64-autolanding/127
    SUCCESS: http://jenkins.qa.ubuntu.com/job/webbrowser-app-saucy-armhf-autolanding/127
        deb: http://jenkins.qa.ubuntu.com/job/webbrowser-app-saucy-armhf-autolanding/127/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/webbrowser-app-saucy-i386-autolanding/127
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-saucy-i386/3433
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-saucy-i386/3433/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-runner-saucy/2894
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-saucy-armhf/951
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-saucy-armhf/951/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-runner-maguro/794
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-runner-mako/806

review: Needs Fixing (continuous-integration)

[Alexandre Abreu (abreu-alexandre)]

review: Approve

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:297
http://jenkins.qa.ubuntu.com/job/webbrowser-app-ci/326/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-saucy/3422
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-touch/950
    SUCCESS: http://jenkins.qa.ubuntu.com/job/webbrowser-app-saucy-amd64-ci/209
    SUCCESS: http://jenkins.qa.ubuntu.com/job/webbrowser-app-saucy-armhf-ci/209
        deb: http://jenkins.qa.ubuntu.com/job/webbrowser-app-saucy-armhf-ci/209/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/webbrowser-app-saucy-i386-ci/209
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-saucy-i386/3434
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-saucy-i386/3434/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-runner-saucy/2895
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-saucy-armhf/952
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-saucy-armhf/952/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-runner-maguro/795
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-runner-mako/807

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

review: Approve (continuous-integration)

=== modified file 'src/Ubuntu/Components/Extras/Browser/UbuntuWebView.qml'
--- src/Ubuntu/Components/Extras/Browser/UbuntuWebView.qml	2013-08-16 15:14:32 +0000
+++ src/Ubuntu/Components/Extras/Browser/UbuntuWebView.qml	2013-09-13 22:57:01 +0000
@@ -17,7 +17,7 @@
  */
 
 import QtQuick 2.0
-import QtWebKit 3.0
+import QtWebKit 3.1
 import QtWebKit.experimental 1.0
 import Ubuntu.Components 0.1
 import Ubuntu.Components.Extras.Browser 0.1
@@ -41,13 +41,26 @@
 
     property real scale: experimental.test.contentsScale * experimental.test.devicePixelRatio
 
+    /**
+     * Client overridable function called before the default treatment of a
+     *  valid navigation request. This function can stop the navigation request
+     *  if it sets the 'action' field of the request to IgnoreRequest.
+     *
+     */
+    function navigationRequestedDelegate(request) { }
+
     UserAgent {
         id: userAgent
     }
     experimental.userAgent: userAgent.defaultUA
     onNavigationRequested: {
+        request.action = WebView.AcceptRequest;
+
+        navigationRequestedDelegate (request);
+        if (request.action === WebView.IgnoreRequest)
+            return;
+
         _webview.experimental.userAgent = userAgent.getUAString(request.url)
-        request.action = WebView.AcceptRequest
     }
 
     experimental.preferences.navigatorQtObjectEnabled: true

=== modified file 'src/app/Browser.qml'
--- src/app/Browser.qml	2013-09-03 20:28:36 +0000
+++ src/app/Browser.qml	2013-09-13 22:57:01 +0000
@@ -17,7 +17,7 @@
  */
 
 import QtQuick 2.0
-import QtWebKit 3.0
+import QtWebKit 3.1
 import QtWebKit.experimental 1.0
 import Ubuntu.Components 0.1
 import Ubuntu.Components.Extras.Browser 0.1
@@ -32,6 +32,8 @@
     property real qtwebkitdpr
     property bool developerExtrasEnabled: false
 
+    property var webappUrlPatterns: null
+
     property bool webapp: false
     property string webappName: ""
 
@@ -305,6 +307,7 @@
             id: webappsComponent
 
             UnityWebApps.UnityWebApps {
+                id: webapps
                 name: browser.webappName
                 bindee: tabsModel.currentWebview
                 actionsContext: browser.actionManager.globalContext
@@ -369,6 +372,38 @@
                 }
             }
 
+            function navigationRequestedDelegate(request) {
+                if (! request.isMainFrame) {
+                    request.action = WebView.AcceptRequest;
+                    return;
+                }
+
+                var action = WebView.AcceptRequest;
+                var url = request.url.toString();
+
+                // The list of url patterns defined by the webapp takes precedence over command line
+                if (webapp && isRunningAsANamedWebapp() && webapps.model && webapps.model.exists(webapps.name)) {
+                    if ( ! webapps.model.doesUrlMatchesWebapp(webapps.name, url)) {
+                        action = WebView.IgnoreRequest;
+                    }
+                }
+                else if (browser.webappUrlPatterns && browser.webappUrlPatterns.length !== 0) {
+                    action = WebView.IgnoreRequest;
+                    for (var i = 0; i < browser.webappUrlPatterns.length; ++i) {
+                        var pattern = browser.webappUrlPatterns[i];
+                        if (url.match(pattern)) {
+                            action = WebView.AcceptRequest;
+                            break;
+                        }
+                    }
+                }
+
+                request.action = action;
+                if (action === WebView.IgnoreRequest) {
+                    Qt.openUrlExternally(url);
+                }
+            }
+
             onNewTabRequested: browser.newTab(url, true)
 
             // Small shim needed when running as a webapp to wire-up connections
@@ -377,7 +412,8 @@
             // component as a way to bind to a webview lookalike without
             // reaching out directly to its internals (see it as an interface).
             function getUnityWebappsProxies() {
-                return UnityWebAppsUtils.makeProxiesForQtWebViewBindee(webview)
+                var handlers = {};
+                return UnityWebAppsUtils.makeProxiesForQtWebViewBindee(webview, handlers)
             }
         }
     }

=== modified file 'src/app/commandline-parser.cpp'
--- src/app/commandline-parser.cpp	2013-09-06 16:29:55 +0000
+++ src/app/commandline-parser.cpp	2013-09-13 22:57:01 +0000
@@ -54,6 +54,19 @@
                     m_fullscreen = true;
                 } else if (argument == "--inspector") {
                     m_remoteInspector = true;
+                } else if (argument.startsWith("--webappUrlPatterns=")) {
+                    QString tail = argument.split("--webappUrlPatterns=")[1];
+                    if (!tail.isEmpty()) {
+                        QStringList includePatterns = tail.split(",");
+                        Q_FOREACH(const QString & includePattern, includePatterns)
+                        {
+                            QString url = includePattern.trimmed();
+                            if ( ! url.isEmpty())
+                            {
+                                m_webappUrlPatterns.append(url);
+                            }
+                        }
+                    }
                 } else if (argument == "--enable-back-forward") {
                     m_chromeFlags |= BACK_FORWARD_BUTTONS;
                 } else if (argument == "--enable-activity") {
@@ -121,6 +134,7 @@
     out << "  --inspector            run a remote inspector on port " << REMOTE_INSPECTOR_PORT << endl;
     out << "  --webapp[=name]        launch the browser as a webapp trying to match it by name with an installed webapp integration script (if any)" << endl;
     out << "  --app-id=APP_ID        run the application with a specific APP_ID" << endl;
+    out << "  --webappUrlPatterns=url-patterns  when running as a webapp (see --webapp), list of ',' separated url patterns (wildcard based) that the webapp can navigate to" << endl;
     out << "Chrome options (if none specified, the whole chrome is enabled by default):" << endl;
     out << "  --chromeless           do not display any chrome (web application mode), if set it overrides the other chrome options" << endl;
     out << "  --enable-back-forward  enable the display of the back and forward buttons" << endl;
@@ -172,3 +186,8 @@
 {
     return m_webappName;
 }
+
+QStringList CommandLineParser::webappUrlPatterns() const
+{
+    return m_webappUrlPatterns;
+}

=== modified file 'src/app/commandline-parser.h'
--- src/app/commandline-parser.h	2013-09-06 16:29:55 +0000
+++ src/app/commandline-parser.h	2013-09-13 22:57:01 +0000
@@ -22,6 +22,7 @@
 // Qt
 #include <QtCore/QObject>
 #include <QtCore/QString>
+#include <QtCore/QStringList>
 #include <QtCore/QUrl>
 
 class CommandLineParser : public QObject
@@ -60,9 +61,9 @@
 
     QString appId() const;
 
+    QStringList webappUrlPatterns() const;
     ChromeElementFlags chromeFlags() const;
 
-
 private:
     bool m_help;
     bool m_fullscreen;
@@ -71,6 +72,7 @@
     bool m_webapp;
     QString m_webappName;
     QString m_appid;
+    QStringList m_webappUrlPatterns;
     ChromeElementFlags m_chromeFlags;
 };
 

=== modified file 'src/app/webbrowser-app.cpp'
--- src/app/webbrowser-app.cpp	2013-09-06 16:29:55 +0000
+++ src/app/webbrowser-app.cpp	2013-09-13 22:57:01 +0000
@@ -120,6 +120,16 @@
         browser->setProperty("activityButtonVisible", activityButtonVisible);
     }
 
+    QStringList urlPatterns = m_arguments->webappUrlPatterns();
+    if ( ! urlPatterns.isEmpty())
+    {
+        for (int i = 0; i < urlPatterns.count(); ++i)
+        {
+            urlPatterns[i].replace("*", "[^ ]*");
+        }
+        browser->setProperty("webappUrlPatterns", urlPatterns);
+    }
+
     // Set the desired pixel ratio (not needed once we use Qt's way of calculating
     // the proper pixel ratio by device/screen)
     float webkitDpr = getQtWebkitDpr();

=== modified file 'src/app/webbrowser-app.qml'
--- src/app/webbrowser-app.qml	2013-09-03 20:28:36 +0000
+++ src/app/webbrowser-app.qml	2013-09-13 22:57:01 +0000
@@ -25,6 +25,8 @@
     property alias qtwebkitdpr: browser.qtwebkitdpr
     property alias developerExtrasEnabled: browser.developerExtrasEnabled
 
+    property alias webappUrlPatterns: browser.webappUrlPatterns
+
     property alias backForwardButtonsVisible: browser.backForwardButtonsVisible
     property alias activityButtonVisible: browser.activityButtonVisible
     property alias addressBarVisible: browser.addressBarVisible

=== modified file 'tests/unittests/commandline-parser/tst_CommandLineParserTests.cpp'
--- tests/unittests/commandline-parser/tst_CommandLineParserTests.cpp	2013-09-06 16:13:18 +0000
+++ tests/unittests/commandline-parser/tst_CommandLineParserTests.cpp	2013-09-13 22:57:01 +0000
@@ -205,13 +205,42 @@
         QCOMPARE(CommandLineParser(args).webappName(), webappName);
     }
 
+    void shouldUseIncludes_data()
+    {
+        QTest::addColumn<QStringList>("args");
+        QTest::addColumn<QStringList>("patterns");
+
+        QString BINARY("webbrowser-app");
+        QString INCLUDE_PATTERN("http://www.ubuntu.*/*");
+        QString INCLUDE_PATTERN2("http://www.bbc.*/*");
+
+        QTest::newRow("no switch") << (QStringList() << BINARY) << QStringList();
+
+        QTest::newRow("switch only") << (QStringList() << BINARY << "--webappUrlPatterns") << QStringList();
+        QTest::newRow("empty switch") << (QStringList() << BINARY << "--webappUrlPatterns=") << QStringList();
+
+        QTest::newRow("switch and one pattern")
+                << (QStringList() << BINARY << (QString("--webappUrlPatterns=") + INCLUDE_PATTERN))
+                << (QStringList() << INCLUDE_PATTERN);
+
+        QTest::newRow("switch and multiple trimmed pattern")
+                << (QStringList() << BINARY << (QString("--webappUrlPatterns=") + INCLUDE_PATTERN + " , " + INCLUDE_PATTERN2 + " ,  "))
+                << (QStringList() << INCLUDE_PATTERN << INCLUDE_PATTERN2);
+    }
+
+    void shouldUseIncludes()
+    {
+        QFETCH(QStringList, args);
+        QFETCH(QStringList, patterns);
+        QCOMPARE(CommandLineParser(args).webappUrlPatterns(), patterns);
+    }
+
     void shouldUseChrome_data()
     {
         QTest::addColumn<QStringList>("args");
         QTest::addColumn<uint>("chrome");
 
         QString BINARY("webbrowser-app");
-
         QString CHROME_BACK_FORWARD("--enable-back-forward");
         QString CHROME_ACTIVIY("--enable-activity");
         QString CHROME_ADDRESS_BAR("--enable-addressbar");