webbrowser-app

Merge lp:~abreu-alexandre/webbrowser-app/handle-simplified-webapp-definition into lp:webbrowser-app

  1. handle-simplified-webapp-definition
  2. Merge into trunk
Proposed by Alexandre Abreu
Status: Merged
Approved by: Olivier Tilloy
Approved revision: 645
Merged at revision: 649
Proposed branch: lp:~abreu-alexandre/webbrowser-app/handle-simplified-webapp-definition
Merge into: lp:webbrowser-app
Diff against target: 44 lines (+13/-2)
2 files modified
src/app/webcontainer/webapp-container.cpp (+3/-1)
src/app/webcontainer/webapp-container.qml (+10/-1)
To merge this branch: bzr merge lp:~abreu-alexandre/webbrowser-app/handle-simplified-webapp-definition
Reviewer Review Type Date Requested Status
PS Jenkins bot continuous-integration Approve
Olivier Tilloy Approve
Review via email: mp+228562@code.launchpad.net

Commit message

Handle simplified webapp definition

Description of the change

Handle simplified webapp definition

To post a comment you must log in.
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

PASSED: Continuous integration, rev:642
http://jenkins.qa.ubuntu.com/job/webbrowser-app-ci/982/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-utopic-touch/2607
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-utopic/2089
    SUCCESS: http://jenkins.qa.ubuntu.com/job/webbrowser-app-utopic-amd64-ci/181
    SUCCESS: http://jenkins.qa.ubuntu.com/job/webbrowser-app-utopic-armhf-ci/181
        deb: http://jenkins.qa.ubuntu.com/job/webbrowser-app-utopic-armhf-ci/181/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/webbrowser-app-utopic-i386-ci/181
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-runner-mako/2740
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-armhf/3850
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-armhf/3850/artifact/work/output/*zip*/output.zip
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/10561
    SUCCESS: http://jenkins.qa.ubuntu.com/job/autopilot-testrunner-otto-utopic/1745
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-amd64/2342
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-amd64/2342/artifact/work/output/*zip*/output.zip

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

review: Approve (continuous-integration)
Revision history for this message
Olivier Tilloy (osomon) wrote :

8 - } else {
9 - return false;

I’m not sure I understand what a simplified webapp definition is: what happens if neither a webapp name nor a URL is passed on the command line? How does the container know where to browse?

review: Needs Information
Revision history for this message
Olivier Tilloy (osomon) wrote :

34 + if (webappName !== browser.webappName) {

This will never evaluate to true, as it’s being evaluated in the scope of the 'browser' item, so (webappName == this.webappName == browser.webappName).

review: Needs Fixing
Revision history for this message
Alexandre Abreu (abreu-alexandre) wrote :

> 8 - } else {
> 9 - return false;
>
> I’m not sure I understand what a simplified webapp definition is: what happens
> if neither a webapp name nor a URL is passed on the command line? How does the
> container know where to browse?

It knows (and always have known, the command line bit only being one very raw way to do it), through the unitywebapp qml component, that is the one that drives browsing to the homepage URL. This is the way it works on the desktop where webapps are more complex, and as the phone ones are getting more complex, we just reusing a mechanism that already works,

Revision history for this message
Alexandre Abreu (abreu-alexandre) wrote :

> 34 + if (webappName !== browser.webappName) {
>
> This will never evaluate to true, as it’s being evaluated in the scope of the
> 'browser' item, so (webappName == this.webappName == browser.webappName).

Totally right (obviously), I mistook 'browser' id for the 'root' one

lp:~abreu-alexandre/webbrowser-app/handle-simplified-webapp-definition updated
643. By Alexandre Abreu

fix

644. By Alexandre Abreu

add comment

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

PASSED: Continuous integration, rev:643
http://jenkins.qa.ubuntu.com/job/webbrowser-app-ci/995/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-utopic-touch/2874
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-utopic/2283
    SUCCESS: http://jenkins.qa.ubuntu.com/job/webbrowser-app-utopic-amd64-ci/194
    SUCCESS: http://jenkins.qa.ubuntu.com/job/webbrowser-app-utopic-armhf-ci/194
        deb: http://jenkins.qa.ubuntu.com/job/webbrowser-app-utopic-armhf-ci/194/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/webbrowser-app-utopic-i386-ci/194
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-runner-mako/2943
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-armhf/4117
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-armhf/4117/artifact/work/output/*zip*/output.zip
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/10814
    SUCCESS: http://jenkins.qa.ubuntu.com/job/autopilot-testrunner-otto-utopic/1888
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-amd64/2547
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-amd64/2547/artifact/work/output/*zip*/output.zip

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

review: Approve (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

PASSED: Continuous integration, rev:644
http://jenkins.qa.ubuntu.com/job/webbrowser-app-ci/996/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-utopic-touch/2878
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-utopic/2286
    SUCCESS: http://jenkins.qa.ubuntu.com/job/webbrowser-app-utopic-amd64-ci/195
    SUCCESS: http://jenkins.qa.ubuntu.com/job/webbrowser-app-utopic-armhf-ci/195
        deb: http://jenkins.qa.ubuntu.com/job/webbrowser-app-utopic-armhf-ci/195/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/webbrowser-app-utopic-i386-ci/195
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-runner-mako/2946
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-armhf/4121
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-armhf/4121/artifact/work/output/*zip*/output.zip
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/10818
    SUCCESS: http://jenkins.qa.ubuntu.com/job/autopilot-testrunner-otto-utopic/1892
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-amd64/2550
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-amd64/2550/artifact/work/output/*zip*/output.zip

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

review: Approve (continuous-integration)
Revision history for this message
Olivier Tilloy (osomon) wrote :

24 title: {
25 + return getWindowTitle()
26 + }

Can be written:

    title: getWindowTitle()

Revision history for this message
Olivier Tilloy (osomon) wrote :

40 + browser.title = getWindowTitle();

As far as I can tell, 'browser' doesn’t have a title property, did you mean to set 'root.title'?

review: Needs Fixing
lp:~abreu-alexandre/webbrowser-app/handle-simplified-webapp-definition updated
645. By Alexandre Abreu

fixes

Revision history for this message
Alexandre Abreu (abreu-alexandre) wrote :

done

Revision history for this message
Olivier Tilloy (osomon) wrote :

Looks good now. Not functionally tested this specific use case, I’ll trust you that it works as expected.

review: Approve
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

PASSED: Continuous integration, rev:645
http://jenkins.qa.ubuntu.com/job/webbrowser-app-ci/997/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-utopic-touch/2901
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-utopic/2294
    SUCCESS: http://jenkins.qa.ubuntu.com/job/webbrowser-app-utopic-amd64-ci/196
    SUCCESS: http://jenkins.qa.ubuntu.com/job/webbrowser-app-utopic-armhf-ci/196
        deb: http://jenkins.qa.ubuntu.com/job/webbrowser-app-utopic-armhf-ci/196/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/webbrowser-app-utopic-i386-ci/196
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-runner-mako/2966
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-armhf/4144
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-armhf/4144/artifact/work/output/*zip*/output.zip
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/10845
    SUCCESS: http://jenkins.qa.ubuntu.com/job/autopilot-testrunner-otto-utopic/1897
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-amd64/2559
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-amd64/2559/artifact/work/output/*zip*/output.zip

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

review: Approve (continuous-integration)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'src/app/webcontainer/webapp-container.cpp'
2--- src/app/webcontainer/webapp-container.cpp 2014-07-17 09:56:20 +0000
3+++ src/app/webcontainer/webapp-container.cpp 2014-08-01 12:21:10 +0000
4@@ -138,9 +138,11 @@
5 QList<QUrl> urls = this->urls();
6 if (!urls.isEmpty()) {
7 m_window->setProperty("url", urls.last());
8- } else {
9+ } else if (m_webappModelSearchPath.isEmpty()) {
10 return false;
11 }
12+ // Otherwise, assume that the homepage will come from a locally defined
13+ // webapp-properties.json file pulled from the webapp model element.
14 }
15
16 m_component->completeCreate();
17
18=== modified file 'src/app/webcontainer/webapp-container.qml'
19--- src/app/webcontainer/webapp-container.qml 2014-07-29 21:51:07 +0000
20+++ src/app/webcontainer/webapp-container.qml 2014-08-01 12:21:10 +0000
21@@ -45,7 +45,9 @@
22 width: 800
23 height: 600
24
25- title: {
26+ title: getWindowTitle()
27+
28+ function getWindowTitle() {
29 if (typeof(webappName) === 'string' && webappName.length !== 0) {
30 return webappName
31 } else if (webappPageComponentLoader.item &&
32@@ -83,6 +85,13 @@
33 webbrowserWindow: webbrowserWindowProxy
34
35 Component.onCompleted: i18n.domain = "webbrowser-app"
36+
37+ onWebappNameChanged: {
38+ if (root.webappName !== browser.webappName) {
39+ root.webappName = browser.webappName;
40+ root.title = getWindowTitle();
41+ }
42+ }
43 }
44 }
45

Subscribers

People subscribed via source and target branches

to status/vote changes: