Merge lp:~michael-sheldon/webbrowser-app/file-upload into lp:webbrowser-app

Proposed by Michael Sheldon
Status: Merged
Approved by: Olivier Tilloy
Approved revision: 455
Merged at revision: 493
Proposed branch: lp:~michael-sheldon/webbrowser-app/file-upload
Merge into: lp:webbrowser-app
Diff against target: 501 lines (+352/-26)
9 files modified
debian/control (+3/-1)
po/webbrowser-app.pot (+11/-3)
src/app/ContentPickerDialog.qml (+98/-0)
src/app/FilePickerDialog.qml (+44/-0)
src/app/WebViewImpl.qml (+6/-0)
src/app/browserapplication.cpp (+1/-20)
src/app/browserapplication.h (+5/-2)
tests/autopilot/webbrowser_app/tests/http_server.py (+18/-0)
tests/autopilot/webbrowser_app/tests/test_content_pick.py (+166/-0)
To merge this branch: bzr merge lp:~michael-sheldon/webbrowser-app/file-upload
Reviewer Review Type Date Requested Status
Olivier Tilloy Approve
PS Jenkins bot continuous-integration Needs Fixing
Ken VanDine Needs Fixing
Review via email: mp+212605@code.launchpad.net

Commit message

Add support for file upload via content-hub

Description of the change

Adds support for file upload via content-hub.

To post a comment you must log in.
Revision history for this message
Michael Sheldon (michael-sheldon) wrote :

Are there any related MPs required for this MP to build/function as expected? Please list.

 * No

Is your branch in sync with latest trunk (e.g. bzr pull lp:trunk -> no changes)

 * Yes

Did you perform an exploratory manual test run of your code change and any related functionality on device or emulator?

 * Yes

Did you successfully run all tests found in your component's Test Plan (https://wiki.ubuntu.com/Process/Merges/TestPlan/webbrowser-app) on device or emulator?

 * Yes

If you changed the UI, was the change specified/approved by design?

 * Added dialog containing a ContentPeerPicker (specified in https://docs.google.com/a/canonical.com/document/d/1trse15NokU8IJ5lm3BnUi7oMNTCkUnYNHeAHZdtzFoQ)

If you changed the packaging (debian), did you subscribe a core-dev to this MP?

 * Added qtdeclarative5-ubuntu-content0.1 to dependencies, subscribed: ken-vandine

Revision history for this message
Ken VanDine (ken-vandine) wrote :

I only reviewed the packaging changes, just one nitpick. In debian/control, please keep the package lists for depends sorted alphabetically.

review: Needs Fixing
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Approve (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Approve (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Approve (continuous-integration)
Revision history for this message
Olivier Tilloy (osomon) wrote :

Testing on my desktop, and I don’t have any app registered in the content hub, so the picker shows up empty, and I’m seeing the following two errors on the console:

file:///usr/lib/x86_64-linux-gnu/qt5/qml/Ubuntu/Content/ResponsiveGridView.qml:37: TypeError: Cannot read property 'count' of undefined
file:///home/osomon/dev/phablet/browser/webbrowser-app/src/app/ContentPickerDialog.qml:60:17: Unable to assign [undefined] to QObject*

That’s a corner case of course, but it would be nice if it could be handled gracefully (ideally some message to the user, which would be something to discuss with design, but even without that, at least make the code robust enough to not spit out errors when the list of handlers is empty).

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

When clicking on the cancel button of the picker, I get this error:

file:///home/osomon/dev/phablet/browser/webbrowser-app/src/app/ContentPickerDialog.qml:54: TypeError: Property 'dismiss' of object oxide::qquick::FilePickerContext(0x32d1b30) is not a function

you will need to s/dismiss()/reject()/ in src/app/ContentPickerDialog.qml.

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

34 + * Copyright 2013 Canonical Ltd.

This should probably be 2014

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

Can you update the license header of tests/autopilot/webbrowser_app/tests/test_content_pick.py to match that of other files in the same directory?

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

A style nitpick: I usually do not end lines with semi-colons for javascript code in QML files.
For consistency, would you mind doing that too in src/app/ContentPickerDialog.qml ?

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

135 + Loader {
136 + id: peerModelLoader
137 + active: false
138 + sourceComponent: ContentPeerModel {
139 + contentType: ContentType.All
140 + handler: ContentHandler.Source
141 + }
142 + }

Does this need to be a separate component? I don’t see it reused anywhere else, maybe the customPeerModelLoader property of the ContentPickerDialog instance could instantiate it directly?

Revision history for this message
Michael Sheldon (michael-sheldon) wrote :

The Loader is separate because the ContentPickerDialog and gets destroyed when the dialog is closed, keeping it separate allows us to avoid having to requery the content-hub for peers every time the dialog is displayed (which is less of an issue now the dbus speed problems are resolved, but could become important again when there are a lot of peers).

(The ContentPeerPicker component will create a model itself when a custom model isn't provided, so most apps don't need to worry about this sort of thing).

Revision history for this message
Michael Sheldon (michael-sheldon) wrote :

file:///home/osomon/dev/phablet/browser/webbrowser-app/src/app/ContentPickerDialog.qml:60:17: Unable to assign [undefined] to QObject*

 Should be fixed now.

file:///usr/lib/x86_64-linux-gnu/qt5/qml/Ubuntu/Content/ResponsiveGridView.qml:37: TypeError: Cannot read property 'count' of undefined

 This one appears to have been introduced on the content-hub side when using a custom loader, so I'll fix that in a content-hub MR.

 Displaying messages when no peers are found should probably also happen in the content-hub (the main ContentPeerPicker element comes from content-hub, ContentPickerDialog is just a wrapper around it to make it usable within the browser).

Revision history for this message
Michael Sheldon (michael-sheldon) wrote :

Just so you're aware, there'll also be a few warnings about height bindings from the ContentTransferHint when a transfer is in progress. This is due to the ContentTransferHint being a dialog which is then being embedded in another dialog (the ContentPickerDialog), but I'm not sure there's any way around this due to the browser needing the file picker to be a dialog.

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

FAILED: Continuous integration, rev:430
http://jenkins.qa.ubuntu.com/job/webbrowser-app-ci/711/
Executed test runs:
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-trusty/4625
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-trusty-touch/4112/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/webbrowser-app-trusty-amd64-ci/213
    SUCCESS: http://jenkins.qa.ubuntu.com/job/webbrowser-app-trusty-armhf-ci/213
        deb: http://jenkins.qa.ubuntu.com/job/webbrowser-app-trusty-armhf-ci/213/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/webbrowser-app-trusty-i386-ci/213
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/autopilot-testrunner-otto-trusty/3982
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-amd64/4749
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-amd64/4749/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-armhf/4218
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-armhf/4218/artifact/work/output/*zip*/output.zip
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-runner-mako/6347/console
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/5759

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

review: Needs Fixing (continuous-integration)
Revision history for this message
Michael Sheldon (michael-sheldon) wrote :

> file:///usr/lib/x86_64-linux-
> gnu/qt5/qml/Ubuntu/Content/ResponsiveGridView.qml:37: TypeError: Cannot read
> property 'count' of undefined

 This is now fixed in https://code.launchpad.net/~michael-sheldon/content-hub/fix-grid-view-loader/+merge/214835. It shouldn't prevent the file picker functionality working normally before that branch is merged though, as it just effects the clipping on the grid view.

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

FAILED: Continuous integration, rev:434
http://jenkins.qa.ubuntu.com/job/webbrowser-app-ci/715/
Executed test runs:
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-trusty/4645
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-trusty-touch/4118/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/webbrowser-app-trusty-amd64-ci/217
    SUCCESS: http://jenkins.qa.ubuntu.com/job/webbrowser-app-trusty-armhf-ci/217
        deb: http://jenkins.qa.ubuntu.com/job/webbrowser-app-trusty-armhf-ci/217/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/webbrowser-app-trusty-i386-ci/217
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/autopilot-testrunner-otto-trusty/4002
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-amd64/4769
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-amd64/4769/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-armhf/4233
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-armhf/4233/artifact/work/output/*zip*/output.zip
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-runner-mako/6353/console
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/5785

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

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

> The Loader is separate because the ContentPickerDialog and gets destroyed when
> the dialog is closed, keeping it separate allows us to avoid having to requery
> the content-hub for peers every time the dialog is displayed (which is less of
> an issue now the dbus speed problems are resolved, but could become important
> again when there are a lot of peers).
>
> (The ContentPeerPicker component will create a model itself when a custom
> model isn't provided, so most apps don't need to worry about this sort of
> thing).

That makes sense, thanks for explaining. Would you mind adding a comment in the Loader instance to explain that? It’s not immediately obvious from a quick peak at the code, I wouldn’t want this optimization to disappear in a future refactoring.

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

FAILED: Continuous integration, rev:435
http://jenkins.qa.ubuntu.com/job/webbrowser-app-ci/719/
Executed test runs:
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-trusty/4661
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-trusty-touch/4123/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/webbrowser-app-trusty-amd64-ci/221
    SUCCESS: http://jenkins.qa.ubuntu.com/job/webbrowser-app-trusty-armhf-ci/221
        deb: http://jenkins.qa.ubuntu.com/job/webbrowser-app-trusty-armhf-ci/221/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/webbrowser-app-trusty-i386-ci/221
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/autopilot-testrunner-otto-trusty/4017
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-amd64/4788
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-amd64/4788/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-armhf/4248
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-armhf/4248/artifact/work/output/*zip*/output.zip
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-runner-mako/6357/console
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/5804

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

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

I’m seeing an issue that I can reliably reproduce:

 - open a page that has an input="file" widget (I’m using http://www.html5rocks.com/en/tutorials/file/dndfiles/), click the button to pick content
 - choose the gallery app, select a picture, then click "Pick", you’re back to the browser and the picture has been selected in the input widget
 - click the button again to e.g. pick another picture
 - choose the gallery app

Expected result: the gallery app opens and I can pick a picture
Current result: I get a small dialog with a spinner that never goes away (if however in the meantime I went to the dash and manually closed the gallery app, then the issue cannot be observed)

Revision history for this message
Michael Sheldon (michael-sheldon) wrote :

> I’m seeing an issue that I can reliably reproduce:
>
> - open a page that has an input="file" widget (I’m using
> http://www.html5rocks.com/en/tutorials/file/dndfiles/), click the button to
> pick content
> - choose the gallery app, select a picture, then click "Pick", you’re back to
> the browser and the picture has been selected in the input widget
> - click the button again to e.g. pick another picture
> - choose the gallery app
>
> Expected result: the gallery app opens and I can pick a picture
> Current result: I get a small dialog with a spinner that never goes away (if
> however in the meantime I went to the dash and manually closed the gallery
> app, then the issue cannot be observed)

It looks like most of the content-hub stuff is happening normally and the gallery goes in to picker mode but then just isn't brought to the foreground, I'll check with Ken what should be happening here (that functionality all lies within the content-hub-service), but my guess is that it should be given focus (and I think that happened in the past, so this might be a recent change somewhere else).

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

FAILED: Continuous integration, rev:436
http://jenkins.qa.ubuntu.com/job/webbrowser-app-ci/720/
Executed test runs:
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-trusty/4665
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-trusty-touch/4124/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/webbrowser-app-trusty-amd64-ci/222
    SUCCESS: http://jenkins.qa.ubuntu.com/job/webbrowser-app-trusty-armhf-ci/222
        deb: http://jenkins.qa.ubuntu.com/job/webbrowser-app-trusty-armhf-ci/222/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/webbrowser-app-trusty-i386-ci/222
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/autopilot-testrunner-otto-trusty/4019
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-amd64/4792
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-amd64/4792/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-armhf/4250
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-armhf/4250/artifact/work/output/*zip*/output.zip
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-runner-mako/6359/console
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/5807

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

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

183 + html = '<form action="upload" method="post" enctype="multipart/form-data">'

The "=" should be "+=" here.

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

For the insensitivity workaround, instead of a timer, how about something like that:

    Connections {
        target: Qt.application
        onStateChanged: {
            if ((Qt.application.state === Qt.ApplicationActive) && (selectedItems !== null)) {
                dismissed()
                model.accept(selectedItems)
            }
        }
    }

(you would need to initialize selectedItems to null for this to work)

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

FAILED: Continuous integration, rev:437
http://jenkins.qa.ubuntu.com/job/webbrowser-app-ci/721/
Executed test runs:
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-trusty/4668
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-trusty-touch/4125/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/webbrowser-app-trusty-amd64-ci/223
    SUCCESS: http://jenkins.qa.ubuntu.com/job/webbrowser-app-trusty-armhf-ci/223
        deb: http://jenkins.qa.ubuntu.com/job/webbrowser-app-trusty-armhf-ci/223/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/webbrowser-app-trusty-i386-ci/223
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/autopilot-testrunner-otto-trusty/4021
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-amd64/4795
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-amd64/4795/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-armhf/4253
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-armhf/4253/artifact/work/output/*zip*/output.zip
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-runner-mako/6360/console
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/5810

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

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

FAILED: Continuous integration, rev:439
http://jenkins.qa.ubuntu.com/job/webbrowser-app-ci/722/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-trusty/4671/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-trusty-touch/4126/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/webbrowser-app-trusty-amd64-ci/224
    SUCCESS: http://jenkins.qa.ubuntu.com/job/webbrowser-app-trusty-armhf-ci/224
        deb: http://jenkins.qa.ubuntu.com/job/webbrowser-app-trusty-armhf-ci/224/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/webbrowser-app-trusty-i386-ci/224
    FAILURE: http://jenkins.qa.ubuntu.com/job/autopilot-testrunner-otto-trusty/4026/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-amd64/4798
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-amd64/4798/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-armhf/4255
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-armhf/4255/artifact/work/output/*zip*/output.zip
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-runner-mako/6361/console
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/5815

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

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

FAILED: Continuous integration, rev:440
http://jenkins.qa.ubuntu.com/job/webbrowser-app-ci/724/
Executed test runs:
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-trusty/4676
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-trusty-touch/4128/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/webbrowser-app-trusty-amd64-ci/226
    SUCCESS: http://jenkins.qa.ubuntu.com/job/webbrowser-app-trusty-armhf-ci/226
        deb: http://jenkins.qa.ubuntu.com/job/webbrowser-app-trusty-armhf-ci/226/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/webbrowser-app-trusty-i386-ci/226
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/autopilot-testrunner-otto-trusty/4031
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-amd64/4803
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-amd64/4803/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-armhf/4260
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-armhf/4260/artifact/work/output/*zip*/output.zip
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-runner-mako/6363/console
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/5821

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

review: Needs Fixing (continuous-integration)
Revision history for this message
Bill Filler (bfiller) wrote :

I was thinking about the file picker functionality on the desktop. Assuming we'll want it to operate correctly for webapps on the desktop. We probably don't want the content-hub/peer picker being launched on the desktop, but rather showing the standard File Chooser component so that the user can choose any file of their liking to upload.

What do you guys think?

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

> I was thinking about the file picker functionality on the desktop. Assuming
> we'll want it to operate correctly for webapps on the desktop. We probably
> don't want the content-hub/peer picker being launched on the desktop, but
> rather showing the standard File Chooser component so that the user can choose
> any file of their liking to upload.
>
> What do you guys think?

In a non-unity8 world, I think that’s the desired behaviour indeed.
Note that there’s no such thing as a standard file chooser in QML, one would have to implement one (or expose to QML a standard QFileDialog). We would also need a way to determine which dialog to use (the form factor would work as a temporary solution, but as soon as we have unity8 on the desktop it won’t work anymore).

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

So much for my outdated knowledge of the standard QML components… Here comes the FileDialog component, to the rescue: http://qt-project.org/doc/qt-5/qml-qtquick-dialogs-filedialog.html

Revision history for this message
Bill Filler (bfiller) wrote :

regarding the focus issue with content-hub, this appears to be a shell bug, filed here:
https://bugs.launchpad.net/ubuntu/+source/unity8/+bug/1305128

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

FAILED: Continuous integration, rev:441
http://jenkins.qa.ubuntu.com/job/webbrowser-app-ci/726/
Executed test runs:
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-trusty/4695
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-trusty-touch/4134/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/webbrowser-app-trusty-amd64-ci/228
    SUCCESS: http://jenkins.qa.ubuntu.com/job/webbrowser-app-trusty-armhf-ci/228
        deb: http://jenkins.qa.ubuntu.com/job/webbrowser-app-trusty-armhf-ci/228/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/webbrowser-app-trusty-i386-ci/228
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/autopilot-testrunner-otto-trusty/4051
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-amd64/4822
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-amd64/4822/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-armhf/4278
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-armhf/4278/artifact/work/output/*zip*/output.zip
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-runner-mako/6369/console
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/5842

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

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

FAILED: Continuous integration, rev:442
http://jenkins.qa.ubuntu.com/job/webbrowser-app-ci/728/
Executed test runs:
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-trusty/4703
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-trusty-touch/4137/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/webbrowser-app-trusty-amd64-ci/230
    SUCCESS: http://jenkins.qa.ubuntu.com/job/webbrowser-app-trusty-armhf-ci/230
        deb: http://jenkins.qa.ubuntu.com/job/webbrowser-app-trusty-armhf-ci/230/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/webbrowser-app-trusty-i386-ci/230
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/autopilot-testrunner-otto-trusty/4059
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-amd64/4830
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-amd64/4830/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-armhf/4286
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-armhf/4286/artifact/work/output/*zip*/output.zip
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-runner-mako/6372/console
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/5851

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

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

FAILED: Continuous integration, rev:442
http://jenkins.qa.ubuntu.com/job/webbrowser-app-ci/729/
Executed test runs:
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-trusty/4723
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-trusty-touch/4140/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/webbrowser-app-trusty-amd64-ci/231
    SUCCESS: http://jenkins.qa.ubuntu.com/job/webbrowser-app-trusty-armhf-ci/231
        deb: http://jenkins.qa.ubuntu.com/job/webbrowser-app-trusty-armhf-ci/231/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/webbrowser-app-trusty-i386-ci/231
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/autopilot-testrunner-otto-trusty/4075
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-amd64/4852
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-amd64/4852/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-armhf/4306
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-armhf/4306/artifact/work/output/*zip*/output.zip
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-runner-mako/6375/console
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/5876

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

review: Needs Fixing (continuous-integration)
Revision history for this message
Michael Sheldon (michael-sheldon) wrote :

The last couple of commits provide a slightly more standard file upload dialog when being used on the desktop, however this is currently using the QML file dialog implementation. If we want to use the more common QWidgets based file picker we'd need change the browser from being a QGuiApplication to being a QApplication (and add a dependency on libqt5widgets5), what are people's thoughts on this?

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

297 +#include <QApplication>

Can you make that <QtWidgets/QApplication> for consistency with how we do #includes of Qt headers throughout the project?

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

There’s a trivial conflict in debian/control when merging in the latest trunk, can you please fix it?
While at it, you can bump the required version of liboxideqt-qmlplugin to >= 1.0.0~bzr490 to ensure that we get the filePicker API.

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Olivier Tilloy (osomon) wrote :

I might be wrong, but I don’t think a build dependency on qtdeclarative5-dialogs-plugin is needed (only a runtime one).
Likewise, I don’t think the runtime dependencies that were added to qtdeclarative5-ubuntu-ui-extras-browser-plugin are needed, because the UbuntuWebView component itself is unaffected by your changes.

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

76 + signal dismissed();

extraneous trailing semi-colon here ^

184 + model.accept(selectedFiles)

there’s a mix of tabs and space on this line, breaking the alignment of the code

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

176 + title: i18n.tr("Please choose a file")

Since a new string has been added, the translation template has to be updated. Just run `make webbrowser-app.pot` to regenerate it in your working copy, and commit the changes.

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Olivier Tilloy (osomon) wrote :

20 + qtdeclarative5-ubuntu-content0.1,

I believe this one is not needed either (for the same reason mentioned above), please remove.

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

FAILED: Continuous integration, rev:451
http://jenkins.qa.ubuntu.com/job/webbrowser-app-ci/738/
Executed test runs:
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-trusty/4764
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-trusty-touch/4152/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/webbrowser-app-trusty-amd64-ci/240
    SUCCESS: http://jenkins.qa.ubuntu.com/job/webbrowser-app-trusty-armhf-ci/240
        deb: http://jenkins.qa.ubuntu.com/job/webbrowser-app-trusty-armhf-ci/240/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/webbrowser-app-trusty-i386-ci/240
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/autopilot-testrunner-otto-trusty/4115
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-amd64/4893
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-amd64/4893/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-armhf/4346
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-armhf/4346/artifact/work/output/*zip*/output.zip
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-runner-mako/6385/console
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/5919

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

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

Looks good to me now. Nice job!

review: Approve
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Olivier Tilloy (osomon) wrote :

I just realized that qtdeclarative5-ubuntu-content0.1 is in universe in trusty, whereas webbrowser-app is in main, so it can’t have direct runtime deps in universe.

My suggestion is to simply remove the dependency from debian/control: since it’s only loaded dynamically at run time and only on devices where the content hub is guaranteed to be installed, it won’t make a difference.

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

(for the above to work the ContentPeerModel needs to be moved from WebViewImpl to ContentPickerDialog)

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

Looks good now, thanks Michael!

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'debian/control'
--- debian/control 2014-04-03 14:20:39 +0000
+++ debian/control 2014-04-11 13:15:58 +0000
@@ -31,7 +31,8 @@
31Multi-Arch: foreign31Multi-Arch: foreign
32Depends: ${misc:Depends},32Depends: ${misc:Depends},
33 ${shlibs:Depends},33 ${shlibs:Depends},
34 liboxideqt-qmlplugin (>= 1.0.0~bzr452),34 liboxideqt-qmlplugin (>= 1.0.0~bzr490),
35 qtdeclarative5-dialogs-plugin,
35 qtdeclarative5-qtquick2-plugin,36 qtdeclarative5-qtquick2-plugin,
36 qtdeclarative5-ubuntu-ui-extras-browser-plugin (= ${binary:Version}),37 qtdeclarative5-ubuntu-ui-extras-browser-plugin (= ${binary:Version}),
37 qtdeclarative5-ubuntu-ui-toolkit-plugin,38 qtdeclarative5-ubuntu-ui-toolkit-plugin,
@@ -93,6 +94,7 @@
93 libautopilot-qt (>= 1.4),94 libautopilot-qt (>= 1.4),
94 libqt5test5,95 libqt5test5,
95 ubuntu-ui-toolkit-autopilot,96 ubuntu-ui-toolkit-autopilot,
97 unity8-autopilot,
96 webbrowser-app (>= ${binary:Version}),98 webbrowser-app (>= ${binary:Version}),
97Description: Ubuntu web browser autopilot tests99Description: Ubuntu web browser autopilot tests
98 A lightweight web browser tailored for Ubuntu, based on the Webkit rendering100 A lightweight web browser tailored for Ubuntu, based on the Webkit rendering
99101
=== modified file 'po/webbrowser-app.pot'
--- po/webbrowser-app.pot 2014-03-26 10:00:18 +0000
+++ po/webbrowser-app.pot 2014-04-11 13:15:58 +0000
@@ -8,7 +8,7 @@
8msgstr ""8msgstr ""
9"Project-Id-Version: webbrowser-app\n"9"Project-Id-Version: webbrowser-app\n"
10"Report-Msgid-Bugs-To: \n"10"Report-Msgid-Bugs-To: \n"
11"POT-Creation-Date: 2014-03-26 10:59+0100\n"11"POT-Creation-Date: 2014-04-10 16:28+0100\n"
12"PO-Revision-Date: YEAR-MO-DA HO:MI+ZONE\n"12"PO-Revision-Date: YEAR-MO-DA HO:MI+ZONE\n"
13"Last-Translator: FULL NAME <EMAIL@ADDRESS>\n"13"Last-Translator: FULL NAME <EMAIL@ADDRESS>\n"
14"Language-Team: LANGUAGE <LL@li.org>\n"14"Language-Team: LANGUAGE <LL@li.org>\n"
@@ -116,6 +116,10 @@
116msgid "Refresh page"116msgid "Refresh page"
117msgstr ""117msgstr ""
118118
119#: src/app/FilePickerDialog.qml:26
120msgid "Please choose a file"
121msgstr ""
122
119#: src/app/PermissionRequest.qml:28123#: src/app/PermissionRequest.qml:28
120msgid "Permission Request"124msgid "Permission Request"
121msgstr ""125msgstr ""
@@ -288,13 +292,13 @@
288292
289#. TRANSLATORS: %1 refers to the current page’s title293#. TRANSLATORS: %1 refers to the current page’s title
290#: src/app/webbrowser/webbrowser-app.qml:35294#: src/app/webbrowser/webbrowser-app.qml:35
291#: src/app/webcontainer/webapp-container.qml:44295#: src/app/webcontainer/webapp-container.qml:45
292#, qt-format296#, qt-format
293msgid "%1 - Ubuntu Web Browser"297msgid "%1 - Ubuntu Web Browser"
294msgstr ""298msgstr ""
295299
296#: src/app/webbrowser/webbrowser-app.qml:37300#: src/app/webbrowser/webbrowser-app.qml:37
297#: src/app/webcontainer/webapp-container.qml:46301#: src/app/webcontainer/webapp-container.qml:47
298msgid "Ubuntu Web Browser"302msgid "Ubuntu Web Browser"
299msgstr ""303msgstr ""
300304
@@ -313,3 +317,7 @@
313#: src/app/webcontainer/AccountsView.qml:38317#: src/app/webcontainer/AccountsView.qml:38
314msgid "Select an account"318msgid "Select an account"
315msgstr ""319msgstr ""
320
321#: src/app/webcontainer/WebViewImplWebkit.qml:63
322msgid "This page wants to know your device’s location."
323msgstr ""
316324
=== added file 'src/app/ContentPickerDialog.qml'
--- src/app/ContentPickerDialog.qml 1970-01-01 00:00:00 +0000
+++ src/app/ContentPickerDialog.qml 2014-04-11 13:15:58 +0000
@@ -0,0 +1,98 @@
1/*
2 * Copyright 2014 Canonical Ltd.
3 *
4 * This file is part of webbrowser-app.
5 *
6 * webbrowser-app is free software; you can redistribute it and/or modify
7 * it under the terms of the GNU General Public License as published by
8 * the Free Software Foundation; version 3.
9 *
10 * webbrowser-app is distributed in the hope that it will be useful,
11 * but WITHOUT ANY WARRANTY; without even the implied warranty of
12 * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
13 * GNU General Public License for more details.
14 *
15 * You should have received a copy of the GNU General Public License
16 * along with this program. If not, see <http://www.gnu.org/licenses/>.
17 */
18
19import QtQuick 2.0
20import Ubuntu.Components 0.1
21import Ubuntu.Components.Popups 0.1 as Popups
22import Ubuntu.Content 0.1
23
24Component {
25 Popups.PopupBase {
26 id: picker
27 property var activeTransfer
28 property var selectedItems
29
30 Rectangle {
31 anchors.fill: parent
32
33 ContentTransferHint {
34 anchors.fill: parent
35 activeTransfer: picker.activeTransfer
36 }
37
38 ContentPeerPicker {
39 id: peerPicker
40 anchors.fill: parent
41 visible: true
42 contentType: ContentType.All
43 handler: ContentHandler.Source
44
45 onPeerSelected: {
46 if (model.allowMultipleFiles) {
47 peer.selectionType = ContentTransfer.Multiple
48 } else {
49 peer.selectionType = ContentTransfer.Single
50 }
51 picker.activeTransfer = peer.request()
52 stateChangeConnection.target = picker.activeTransfer
53 }
54
55 onCancelPressed: {
56 webview.focus = true
57 model.reject()
58 }
59 }
60 }
61
62 Connections {
63 id: stateChangeConnection
64 onStateChanged: {
65 if (picker.activeTransfer.state === ContentTransfer.Charged) {
66 selectedItems = []
67 for(var i in picker.activeTransfer.items) {
68 selectedItems.push(String(picker.activeTransfer.items[i].url).replace("file://", ""))
69 }
70 acceptTimer.running = true
71 }
72 }
73 }
74
75 // FIXME: Work around for browser becoming insensitive to touch events
76 // if the dialog is dismissed while the application is inactive.
77 // Just listening for changes to Qt.application.active doesn't appear
78 // to be enough to resolve this, so it seems that something else needs
79 // to be happening first. As such there's a potential for a race
80 // condition here, although as yet no problem has been encountered.
81 Timer {
82 id: acceptTimer
83 interval: 100
84 repeat: true
85 onTriggered: {
86 if(Qt.application.active) {
87 webview.focus = true
88 model.accept(selectedItems)
89 }
90 }
91 }
92
93 Component.onCompleted: {
94 show()
95 }
96
97 }
98}
099
=== added file 'src/app/FilePickerDialog.qml'
--- src/app/FilePickerDialog.qml 1970-01-01 00:00:00 +0000
+++ src/app/FilePickerDialog.qml 2014-04-11 13:15:58 +0000
@@ -0,0 +1,44 @@
1/*
2 * Copyright 2014 Canonical Ltd.
3 *
4 * This file is part of webbrowser-app.
5 *
6 * webbrowser-app is free software; you can redistribute it and/or modify
7 * it under the terms of the GNU General Public License as published by
8 * the Free Software Foundation; version 3.
9 *
10 * webbrowser-app is distributed in the hope that it will be useful,
11 * but WITHOUT ANY WARRANTY; without even the implied warranty of
12 * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
13 * GNU General Public License for more details.
14 *
15 * You should have received a copy of the GNU General Public License
16 * along with this program. If not, see <http://www.gnu.org/licenses/>.
17 */
18
19import QtQuick 2.0
20import QtQuick.Dialogs 1.0
21import Ubuntu.Components.Popups 0.1 as Popups
22
23Component {
24 Popups.Dialog {
25 FileDialog {
26 id: fileDialog
27 title: i18n.tr("Please choose a file")
28 selectMultiple: model.allowMultipleFiles
29
30 onAccepted: {
31 var selectedFiles = []
32 for(var i in fileDialog.fileUrls) {
33 selectedFiles.push(fileDialog.fileUrls[i].replace("file://", ""))
34 }
35 model.accept(selectedFiles)
36 }
37
38 onRejected: {
39 model.reject()
40 }
41 Component.onCompleted: visible = true
42 }
43 }
44}
045
=== modified file 'src/app/WebViewImpl.qml'
--- src/app/WebViewImpl.qml 2014-04-02 14:33:31 +0000
+++ src/app/WebViewImpl.qml 2014-04-11 13:15:58 +0000
@@ -35,6 +35,12 @@
35 confirmDialog: ConfirmDialog {}35 confirmDialog: ConfirmDialog {}
36 promptDialog: PromptDialog {}36 promptDialog: PromptDialog {}
37 beforeUnloadDialog: BeforeUnloadDialog {}37 beforeUnloadDialog: BeforeUnloadDialog {}
38 filePicker: filePickerLoader.item
39
40 Loader {
41 id: filePickerLoader
42 source: formFactor == "desktop" ? "FilePickerDialog.qml" : "ContentPickerDialog.qml"
43 }
3844
39 /*selectionActions: ActionList {45 /*selectionActions: ActionList {
40 Actions.Copy {46 Actions.Copy {
4147
=== modified file 'src/app/browserapplication.cpp'
--- src/app/browserapplication.cpp 2014-04-02 15:15:27 +0000
+++ src/app/browserapplication.cpp 2014-04-11 13:15:58 +0000
@@ -32,7 +32,7 @@
32#include "webbrowser-window.h"32#include "webbrowser-window.h"
3333
34BrowserApplication::BrowserApplication(int& argc, char** argv)34BrowserApplication::BrowserApplication(int& argc, char** argv)
35 : QGuiApplication(argc, argv)35 : QApplication(argc, argv)
36 , m_engine(0)36 , m_engine(0)
37 , m_window(0)37 , m_window(0)
38 , m_component(0)38 , m_component(0)
@@ -40,25 +40,6 @@
40{40{
41 m_arguments = arguments();41 m_arguments = arguments();
42 m_arguments.removeFirst();42 m_arguments.removeFirst();
43
44 // The testability driver is only loaded by QApplication but not by
45 // QGuiApplication (see https://codereview.qt-project.org/#change,66513).
46 // Let’s load the testability driver on our own.
47 if (m_arguments.contains(QLatin1String("-testability")) ||
48 qgetenv("QT_LOAD_TESTABILITY") == "1") {
49 QLibrary testLib(QLatin1String("qttestability"));
50 if (testLib.load()) {
51 typedef void (*TasInitialize)(void);
52 TasInitialize initFunction = (TasInitialize)testLib.resolve("qt_testability_init");
53 if (initFunction) {
54 initFunction();
55 } else {
56 qCritical("Library qttestability resolve failed!");
57 }
58 } else {
59 qCritical("Library qttestability load failed!");
60 }
61 }
62}43}
6344
64BrowserApplication::~BrowserApplication()45BrowserApplication::~BrowserApplication()
6546
=== modified file 'src/app/browserapplication.h'
--- src/app/browserapplication.h 2014-03-27 17:35:54 +0000
+++ src/app/browserapplication.h 2014-04-11 13:15:58 +0000
@@ -24,14 +24,17 @@
24#include <QtCore/QString>24#include <QtCore/QString>
25#include <QtCore/QStringList>25#include <QtCore/QStringList>
26#include <QtCore/QUrl>26#include <QtCore/QUrl>
27#include <QtGui/QGuiApplication>27#include <QtWidgets/QApplication>
2828
29class QQmlComponent;29class QQmlComponent;
30class QQmlEngine;30class QQmlEngine;
31class QQuickWindow;31class QQuickWindow;
32class WebBrowserWindow;32class WebBrowserWindow;
3333
34class BrowserApplication : public QGuiApplication34// We want the browser to be QApplication based rather than QGuiApplication
35// to provide a widget based file picker on the desktop, rather than the
36// QML fall back picker.
37class BrowserApplication : public QApplication
35{38{
36 Q_OBJECT39 Q_OBJECT
3740
3841
=== modified file 'tests/autopilot/webbrowser_app/tests/http_server.py'
--- tests/autopilot/webbrowser_app/tests/http_server.py 2014-02-25 13:05:27 +0000
+++ tests/autopilot/webbrowser_app/tests/http_server.py 2014-04-11 13:15:58 +0000
@@ -83,6 +83,24 @@
83 html += 'src="/blanktargetlink" />'83 html += 'src="/blanktargetlink" />'
84 html += '</body></html>'84 html += '</body></html>'
85 self.send_html(html)85 self.send_html(html)
86 elif self.path == "/uploadform":
87 # craft a page that accepts clicks anywhere inside its window
88 # and on a click opens up the content picker.
89 # It also pops up an alert with the new content of the file
90 # upload field when it changes
91 self.send_response(200)
92 html = '<html><body style="margin: 0">'
93 html += '<form action="upload" method="post" '
94 html += 'enctype="multipart/form-data">'
95 html += '<input type="file" name="file" id="file" '
96 html += 'onchange="alert(this.value)"><br>'
97 html += '<input type="submit" name="submit" value="Submit">'
98 html += '</form>'
99 html += '<a href="javascript:'
100 html += 'document.getElementById(\'file\').click()">'
101 html += '<div style="height: 100%"></div></a>'
102 html += '</body></html>'
103 self.send_html(html)
86 else:104 else:
87 self.send_error(404)105 self.send_error(404)
88106
89107
=== added file 'tests/autopilot/webbrowser_app/tests/test_content_pick.py'
--- tests/autopilot/webbrowser_app/tests/test_content_pick.py 1970-01-01 00:00:00 +0000
+++ tests/autopilot/webbrowser_app/tests/test_content_pick.py 2014-04-11 13:15:58 +0000
@@ -0,0 +1,166 @@
1# -*- Mode: Python; coding: utf-8; indent-tabs-mode: nil; tab-width: 4 -*-
2#
3# Copyright 2014 Canonical
4#
5# This program is free software: you can redistribute it and/or modify it
6# under the terms of the GNU General Public License version 3, as published
7# by the Free Software Foundation.
8#
9# This program is distributed in the hope that it will be useful,
10# but WITHOUT ANY WARRANTY; without even the implied warranty of
11# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
12# GNU General Public License for more details.
13#
14# You should have received a copy of the GNU General Public License
15# along with this program. If not, see <http://www.gnu.org/licenses/>.
16
17from __future__ import absolute_import
18
19from autopilot.introspection import get_proxy_object_for_existing_process
20from autopilot.matchers import Eventually
21from testtools.matchers import Equals, NotEquals
22from testtools import skip
23from webbrowser_app.tests import StartOpenRemotePageTestCaseBase
24from unity8 import process_helpers as helpers
25from ubuntuuitoolkit import emulators as toolkit_emulators
26import os
27import subprocess
28
29
30@skip("Will not work until bug #1218971 is solved")
31class TestContentPick(StartOpenRemotePageTestCaseBase):
32
33 """Tests that content picking dialog show up."""
34
35 def test_pick_image(self):
36 url = self.base_url + "/uploadform"
37 self.go_to_url(url)
38 self.assert_page_eventually_loaded(url)
39 webview = self.main_window.get_current_webview()
40 self.pointing_device.click_object(webview)
41
42 dialog = self.app.wait_select_single("ContentPickerDialog")
43 self.assertThat(dialog.visible, Equals(True))
44
45
46#@skipIf(model() == 'Desktop', "Phablet only")
47@skip("Currently unable to fetch dynamically created dialogs (bug #1218971)")
48class TestContentPickerIntegration(StartOpenRemotePageTestCaseBase):
49
50 """Tests that the gallery app is brought up to choose image content"""
51
52 def tearDown(self):
53 os.system("pkill gallery-app")
54 os.system("pkill webbrowser-app")
55 super(StartOpenRemotePageTestCaseBase, self).tearDown()
56
57 def get_unity8_proxy_object(self):
58 pid = helpers._get_unity_pid()
59 return get_proxy_object_for_existing_process(pid)
60
61 def get_current_focused_appid(self, unity8):
62 return unity8.select_single("Shell").currentFocusedAppId
63
64 def set_testability_environment_variable(self):
65 """Makes sure every app opened in the environment loads the
66 testability driver."""
67
68 subprocess.check_call([
69 "/sbin/initctl",
70 "set-env",
71 "QT_LOAD_TESTABILITY=1"
72 ])
73
74 def get_app_pid(self, app):
75 """Return the PID of the named app, or -1 if it's not
76 running"""
77
78 try:
79 return int(subprocess.check_output(["pidof", app]).strip())
80 except subprocess.CalledProcessError:
81 return -1
82
83 def wait_app_focused(self, name):
84 """Wait until the app with the specified name is the
85 currently focused one"""
86
87 unity8 = self.get_unity8_proxy_object()
88 self.assertThat(
89 lambda: self.get_current_focused_appid(unity8),
90 Eventually(Equals(name))
91 )
92
93 def test_image_picker_is_gallery(self):
94 """ Tests that the gallery shows up when we are picking
95 images """
96
97 # Go to a page where clicking anywhere equals clicking on the
98 # file selection button of an upload form
99 url = self.base_url + "/uploadform"
100 self.go_to_url(url)
101 self.assert_page_eventually_loaded(url)
102 webview = self.main_window.get_current_webview()
103 self.pointing_device.click_object(webview)
104
105 # Verify that such a click brings up the gallery to select images
106 self.wait_app_focused("gallery-app")
107
108 def test_image_picker_pick_image(self):
109 """ Tests that the we can select an image in the gallery and
110 control will return to the browser with the choosen image
111 picked."""
112
113 # First run the previous test to bring up the content picker
114 self.set_testability_environment_variable()
115 self.test_image_picker_is_gallery()
116
117 # Now wait until the gallery-app process is up.
118 # NOTE: this will not work unless run on a device where unity8 runs in
119 # testability mode. To manually restart unity8 in this mode run from a
120 # python shell:
121 # from unity8 import process_helpers as p
122 # p.restart_unity_with_testability()
123 self.assertThat(lambda: self.get_app_pid("gallery-app"),
124 Eventually(NotEquals(-1)))
125
126 gallery = get_proxy_object_for_existing_process(
127 self.get_app_pid("gallery-app"),
128 emulator_base=toolkit_emulators.UbuntuUIToolkitEmulatorBase
129 )
130
131 # Wait for the gallery UI to completely display
132 view = gallery.wait_select_single("QQuickView")
133 self.assertThat(view.visible, Eventually(Equals(True)))
134
135 # Select the first picture on the picker by clicking on it
136 # NOTE: this is currently failing if there is anything except two
137 # pictures in the gallery (at least on a Maguro device), so I'm
138 # putting a temporary stop to the test here so that it won't break
139 # in Jenkins
140 return
141
142 grid = gallery.wait_select_single("MediaGrid")
143 photo = grid.select_many("OrganicItemInteraction")[0]
144 self.pointing_device.click_object(photo)
145 self.assertThat(photo.isSelected, Eventually(Equals(True)))
146
147 # Now the "Pick" button will be enabled and we click on it
148 button = gallery.select_single("Button", objectName="pickButton")
149 self.assertThat(button.enabled, Eventually(Equals(True)))
150 self.pointing_device.click_object(button)
151
152 # The gallery should close and focus returned to the browser
153 self.wait_app_focused("webbrowser-app")
154
155 # Verify that an image has actually been selected
156 dialog = self.app.wait_select_single("ContentPickerDialog")
157 self.assertThat(dialog.visible, Equals(True))
158 preview = dialog.wait_select_single("QQuickImage",
159 objectName="mediaPreview")
160 self.assertThat(preview.source, Eventually(NotEquals("")))
161
162 # Verify that now we can click the "OK" button and it closes the dialog
163 button = dialog.wait_select_single("Button", objectName="ok")
164 self.assertThat(button.enabled, Eventually(Equals(True)))
165 self.pointing_device.click_object(button)
166 self.assertThat(dialog.visible, Eventually(Equals(False)))

Subscribers

People subscribed via source and target branches

to status/vote changes: