GNOME Bugzilla – Bug 762321
Allow sending file from host to boxes
Last modified: 2018-01-11 10:35:51 UTC
Spice has support for shared folders using WebDAV protocol and a simple and stable API for its usage [0]. The documentation to enable folder sharing with Spice is documented in its manual [1]. [0] http://www.spice-space.org/docs/spice-gtk/SpiceWebdavChannel.html [1] http://www.spice-space.org/docs/manual/#_folder_sharing A daemon in the guest is necessary in order to make a good integration with the desktop. In both Linux and Windows, spice-webdavd can be used which is part of Phodav project [2] [2] https://wiki.gnome.org/phodav Some UI mockups for Boxes can be found here [3] [3] https://raw.githubusercontent.com/gnome-design-team/gnome-mockups/master/boxes/wires/file-sharing.png Remote-viewer can be used as example of client implementing shared folder with Spice like [4] and later patches! [4] https://github.com/SPICE/virt-viewer/commit/73f70899e4a8fae889d0911096ac0c260baf6702
For shared folders (the bottom part of the jimmac's mockup) we already have: https://bugzilla.gnome.org/show_bug.cgi?id=730259 . Let's keep this bug for sending of files..
Created attachment 329853 [details] [review] spice-display: Display file transfer progress Currently, the user has no visual feedback when transfering files to a SPICE remote machine. This isn't a very user friendly behaviour. Let's add a progress bar for each new file transfer and allow the user to cancel a transfer.
Created attachment 330000 [details] [review] display: Add file transfer API Add virtual method transfer_files() and virtual property can_transfer_files, which currently defaults to false for all display types.
Created attachment 330001 [details] [review] spice-display: Implement file transfer
Created attachment 330002 [details] [review] actions-popover: Add "Send file" option
Created attachment 330003 [details] [review] app-window: Add file transfer dialog https://bugzilla.gnome.org/show_bug.cgi?id=681747
Created attachment 330004 [details] [review] app-window: Add file transfer dialog
Created attachment 330005 [details] [review] spice-display: Implement file transfer
Review of attachment 330004 [details] [review]: This is definitely not the final version, as I didn't understand some elements from the mockups.
Review of attachment 330004 [details] [review]: > This is definitely not the final version, as I didn't understand some elements from the mockups. Did you ask jimmac?
Review of attachment 330004 [details] [review]: >Did you ask jimmac? I tried to, but couldn't reach him on IRC.
(In reply to Visarion Alexandru from comment #12) > Review of attachment 330004 [details] [review] [review]: > > >Did you ask jimmac? > I tried to, but couldn't reach him on IRC. In private? Try on #boxes and there is also e-mail. :)
Review of attachment 330004 [details] [review]: After talking to jimmac, it turns out that this should be a final patch. I will have to create another patch, containing a special display when hovering (at drag and drop), but I need to discuss with the GTK folks first, because of some impediments.
Created attachment 331647 [details] [review] display-page: Show file transfer message Currently, when transferring files through drag and drop to a machine with SPICE display, the user isn't aware of the actual destination of the files and doesn't receive any visual feedback. Let's show a message when the user is hovering with a file over a SPICE display, informing him that he is about to send files to the machine and pointing to him the destination of the files.
Created attachment 333633 [details] [review] display: Add file transfer API Add virtual method transfer_files() and virtual property can_transfer_files, which currently defaults to false for all display types. This will help us create a more explicit file transfer between host and SPICE guest machines in following patches.
Created attachment 333634 [details] [review] actions-popover: Add "Send file" option The problem is that with the current file transferring DnD mechanism, users may not be aware of the fact that they are able to send files to guests which have this ability. Using this option, in following patches users will be able to transfer files to a SPICE guest using a more intuitive click of a button.
Created attachment 333635 [details] [review] app-window: Add file transfer dialog Add a dialog as a result of the "Send Files" popover action,through which users can send files to a guest machine which has the ability to receive files (this action is properly enabled/disabled in a previous patch).
Created attachment 333636 [details] [review] display-page: Display file transfer progress Currently, the user has no visual feedback when transfering files to a remote machine and no option to cancel a transfer, which isn't a very user friendly behaviour. Let's create a UI for displaying progress bars on top of a display and to cancel an active transfer. Let's also add a method which allows SPICE displays to make use of this new UI elements, which will be used in the next patch.
Created attachment 333637 [details] [review] display-page: Add support for file transfer feedback Currently, the user has no visual feedback when transfering files to a remote machine and no option to cancel a transfer, which isn't a very user friendly behaviour. Let's create a UI for displaying progress bars on top of a display and to cancel an active transfer. Let's also add a method which allows SPICE displays to make use of this new UI elements, which will be used in the next patch.
Created attachment 333638 [details] [review] spice-display: Show visual feedback on file transfer The previous patch gives the DisplayPage the ability to provide feedback to the user for file transfers and provides a method to achieve this for SPICE. Let's make use of that method and create a progress bar and a cancel option for each new transfer to a SPICE display.
Created attachment 333639 [details] [review] spice-display: Implement file transfer Let's implement the file transfer API for SPICE displays, which will help the user transfer files to a guest with SPICE display in a more user friendly way.
Created attachment 333640 [details] [review] display-page: Show file transfer message Currently, when transferring files through drag and drop, the user isn't aware of the actual destination of the files and doesn't receive any visual feedback. Let's show a message when the user is hovering with a file over a display which can receive files(currently only SPICE), informing him that he is about to send files to the machine and pointing to him the destination of the files.
Review of attachment 333637 [details] [review]: Otherwise a good start for sure. ::: src/display-page.vala @@ +123,3 @@ } + public void add_transfer_progress (Spice.FileTransferTask task) { Display class should not be using/dependent on any particular protocol/tech. That breaks the point of having this base class.
Review of attachment 333633 [details] [review]: looks fine otherwise. ::: src/display.vala @@ +37,3 @@ public virtual void collect_logs (StringBuilder builder) { } + public virtual void transfer_files (SList<string> uris) { Just use GLib.List for consistency.
Nice work. Based solely on the video I've seen* there are a couple of rough edges. [1] In-app notifications The purpose of the notification is to allow to undo a false drop (DND is imprecise and problematic on many input devices). You should not be stacking them. Only one notification should be visible at a time. [2] Progress. You shouldn't stack progress bars. Only one progress bar should be shown. A composite progress of all operations should be reflected in that progressbar. Adding more processes should "move the needle" further form the finish line. In the future a pattern similar to what Files uses might be a better choice. Reused pattern brings consistency. Also avoids the notification overlaid on top of the progress and possibly avpid the visually noisy notification altogether because of the popover allowing to cancel. [3] Overlay The wireframe was perhaps a lacking medium to communicate the visuals. When the Boxes window is ready to accept the drop, the information is supposed to be overlaid over the actual VM content. The content below is toned down in a similar manner the gnome-shell backdrop is toned down when you enter the overview (or more appropriate example -- when the wacom button mapping is shown). Replacing the view altogether to show the window is accepting the drop is functional, but quite distracting. It would be nice to have the overlay in the future. * https://www.youtube.com/watch?v=7Tfhwjrdqkg
why not follow nautilus progression button/popup UI?
> [1] In-app notifications > The purpose of the notification is to allow to undo a false drop (DND is > imprecise and problematic on many input devices). You should not be stacking > them. Only one notification should be visible at a time. When transferring multiple files using the same drag and drop, the Spice.FileTransferTask API just sends a separate signal for each file, so with the current API we can't create a notification for all the files. Either we choose a different behaviour for this case or I create a different Spice.FileTransferTask signal. > [3] Overlay > The wireframe was perhaps a lacking medium to communicate the visuals. When > the Boxes window is ready to accept the drop, the information is supposed to > be overlaid over the actual VM content. The content below is toned down in a > similar manner the gnome-shell backdrop is toned down when you enter the > overview (or more appropriate example -- when the wacom button mapping is > shown). Replacing the view altogether to show the window is accepting the > drop is functional, but quite distracting. It would be nice to have the > overlay in the future. I actually implemented the DND display as an overlay, but I don't know how to tone down the VM content.
Created attachment 342321 [details] [review] display: Add file transfer API Add virtual method transfer_files() and virtual property can_transfer_files, which currently defaults to false for all display types. This will help us create a more explicit file transfer between host and SPICE guest machines in following patches.
Created attachment 342322 [details] [review] actions-popover: Add "Send file" option The problem is that with the current file transferring DnD mechanism, users may not be aware of the fact that they are able to send files to guests which have this ability. Using this option, in following patches users will be able to transfer files to a SPICE guest using a more intuitive click of a button.
Created attachment 342323 [details] [review] app-window: Add file transfer dialog Add a dialog as a result of the "Send Files" popover action,through which users can send files to a guest machine which has the ability to receive files (this action is properly enabled/disabled in a previous patch).
Created attachment 342324 [details] [review] display-page: Add support for transfer progress Currently, the user has no visual feedback when transfering files to a remote machine, which isn't a very user friendly behaviour. Let's create a UI for displaying progress bars on top of a display.
Created attachment 342325 [details] [review] spice-display: Show visual feedback for file transfers Let's show a progress bar which approximates the progress of all the active transfers. Let's also give the user the chance to cancel the ongoing transfers, by prompting him a notification.
Created attachment 342326 [details] [review] spice-display: Implement explicit file transfer Let's implement for SPICE displays the explicit file transfer API introduced in a previous patch, which enables the transfer of files at the push of a button.
Created attachment 342327 [details] [review] spice-display: Implement explicit file transfer Let's implement for SPICE displays the explicit file transfer API introduced in a previous patch, which enables the transfer of files at the push of a button.
Could you please ensure your patches are in order and hence could be cleanly applied to current git master?
Created attachment 342853 [details] [review] display: Add file transfer API Add virtual method transfer_files() and virtual property can_transfer_files, which currently defaults to false for all display types. This will help us create a more explicit file transfer between host and SPICE guest machines in following patches.
Created attachment 342854 [details] [review] actions-popover: Add "Send file" option The problem is that with the current file transferring DnD mechanism, users may not be aware of the fact that they are able to send files to guests which have this ability. Using this option, in following patches users will be able to transfer files to a SPICE guest using a more intuitive click of a button.
Created attachment 342855 [details] [review] app-window: Add file transfer dialog Add a dialog as a result of the "Send Files" popover action,through which users can send files to a guest machine which has the ability to receive files (this action is properly enabled/disabled in a previous patch).
Created attachment 342856 [details] [review] display-page: Add support for transfer progress Currently, the user has no visual feedback when transfering files to a remote machine, which isn't a very user friendly behaviour. Let's create a UI for displaying progress bars on top of a display.
Created attachment 342857 [details] [review] spice-display: Show visual feedback for file transfers Let's show a progress bar which approximates the progress of all the active transfers. Let's also give the user the chance to cancel the ongoing transfers, by prompting him a notification.
Created attachment 342858 [details] [review] spice-display: Implement explicit file transfer Let's implement for SPICE displays the explicit file transfer API introduced in a previous patch, which enables the transfer of files at the push of a button.
Created attachment 342859 [details] [review] display-page: Show file transfer message Currently, when transferring files through drag and drop, the user isn't aware of the actual destination of the files and doesn't receive any visual feedback. Let's show a message when the user is hovering with a file over a display which can receive files(currently only SPICE), informing him that he is about to send files to the machine and pointing to him the destination of the files.
(In reply to Zeeshan Ali (Khattak) from comment #36) > Could you please ensure your patches are in order and hence could be cleanly > applied to current git master? There was a small conflict, but now I am confident that the patches can be cleanly applied to master branch.
(In reply to Visarion Alexandru from comment #44) > (In reply to Zeeshan Ali (Khattak) from comment #36) > > Could you please ensure your patches are in order and hence could be cleanly > > applied to current git master? > > There was a small conflict, but now I am confident that the patches can be > cleanly applied to master branch. Thanks, they do now but now i see a compiler error :( spice-display.vala:351.57-351.80: error: Argument 1: Cannot convert from `uint' to `int' msg = _("Transferring %d files...").printf (transfer_tasks.length ()); ^^^^^^^^^^^^^^^^^^^^^^^^
Created attachment 342913 [details] [review] spice-display: Show visual feedback for file transfers Let's show a progress bar which approximates the progress of all the active transfers. Let's also give the user the chance to cancel the ongoing transfers, by prompting him a notification.
Created attachment 342914 [details] [review] spice-display: Implement explicit file transfer Let's implement for SPICE displays the explicit file transfer API introduced in a previous patch, which enables the transfer of files at the push of a button.
Created attachment 342915 [details] [review] display-page: Show file transfer message Currently, when transferring files through drag and drop, the user isn't aware of the actual destination of the files and doesn't receive any visual feedback. Let's show a message when the user is hovering with a file over a display which can receive files(currently only SPICE), informing him that he is about to send files to the machine and pointing to him the destination of the files.
(In reply to Zeeshan Ali (Khattak) from comment #45) > (In reply to Visarion Alexandru from comment #44) > > (In reply to Zeeshan Ali (Khattak) from comment #36) > > > Could you please ensure your patches are in order and hence could be cleanly > > > applied to current git master? > > > > There was a small conflict, but now I am confident that the patches can be > > cleanly applied to master branch. > > Thanks, they do now but now i see a compiler error :( > > spice-display.vala:351.57-351.80: error: Argument 1: Cannot convert from > `uint' to `int' > msg = _("Transferring %d files...").printf > (transfer_tasks.length ()); > > ^^^^^^^^^^^^^^^^^^^^^^^^ It's a little weird that I wasn't even receiving a warning. Replacing the %d specifier with %u should do the trick. I updated the patches.
(In reply to Visarion Alexandru from comment #49) > (In reply to Zeeshan Ali (Khattak) from comment #45) > > (In reply to Visarion Alexandru from comment #44) > > > (In reply to Zeeshan Ali (Khattak) from comment #36) > > > > Could you please ensure your patches are in order and hence could be cleanly > > > > applied to current git master? > > > > > > There was a small conflict, but now I am confident that the patches can be > > > cleanly applied to master branch. > > > > Thanks, they do now but now i see a compiler error :( > > > > spice-display.vala:351.57-351.80: error: Argument 1: Cannot convert from > > `uint' to `int' > > msg = _("Transferring %d files...").printf > > (transfer_tasks.length ()); > > > > ^^^^^^^^^^^^^^^^^^^^^^^^ > > It's a little weird that I wasn't even receiving a warning. > Replacing the %d specifier with %u should do the trick. I updated the > patches. Thanks it builds now. You'll need to update your vala i guess: jhbuild build vala. :)
Works fine too, just one thing: The notification for cancelling the transfer, should it really timeout?
Created attachment 365808 [details] [review] display: Add file transfer API Add virtual method transfer_files() and virtual property can_transfer_files, which currently defaults to false for all display types. This will help us create a more explicit file transfer between host and SPICE guest machines in following patches.
Created attachment 365809 [details] [review] display: Add file transfer API Add virtual method transfer_files() and virtual property can_transfer_files, which currently defaults to false for all display types. This will help us create a more explicit file transfer between host and SPICE guest machines in following patches.
Created attachment 365810 [details] [review] actions-popover: Add "Send file" option The problem is that with the current file transferring DnD mechanism, users may not be aware of the fact that they are able to send files to guests which have this ability. Using this option, in following patches users will also be able to transfer files to a SPICE guest using a more intuitive click of a button.
Created attachment 365811 [details] [review] app-window: Add file transfer dialog Add a dialog corresponding to the new "Send Files" popover action, through which users can send files to a guest machine which has the ability to receive files.
Created attachment 365812 [details] [review] spice-display: Implement explicit file transfer Let's implement for SPICE displays the explicit file transfer API introduced in a previous patch, which enables the transfer of files at the click of a button.
Created attachment 365814 [details] [review] display-page: Show drag and drop message Currently, when transferring files through DnD, the user isn't aware of the actual destination of the files and doesn't receive any visual feedback. Let's show a message when the user is hovering with a file over a display which can receive files(currently only SPICE), informing him that he is about to send files to the machine and pointing to him the destination of the files.
Created attachment 365815 [details] [review] Add TransferInfoRow Add a custom row which represents a file transfer between the host and a guest machine.
Created attachment 365816 [details] [review] Add TransferPopover Add a popover capable of letting users visualize/manage ongoing transfers between the host and a guest machine, using the previously introduced TransferInfoRow
Created attachment 365817 [details] [review] topbar: Make display_toolbar public We need this for a future patch, so that we can control the behaviour of the file transfer button from the DisplayPage class.
Created attachment 365818 [details] [review] display-toolbar: Add file transfers button Add a button which lets the users view/manage ongoing file transfers between the host and the guest machine corresponding to the current display.
Created attachment 365819 [details] [review] display-page: Enable UI for file transfers Using previous patches, display UI elements when file transfers are ongoing.
Created attachment 365820 [details] [review] display-toolbar: Handle tranfer popover
Created attachment 365821 [details] [review] spice-display: Show progress for SPICE file transfers Display the UI for a new file transfer, using the UI elements implemented in previous patches
The UI has already been approved by jimmac, from a video recording. I have followed Marc-Andre Lureau's advice of making the file transfer UI look and behave as much as possible as the one from Nautilus.
Review of attachment 365809 [details] [review]: sure. Please don't push it until we have reviewed the whole set.
Review of attachment 365810 [details] [review]: all good, just a small pick. ::: src/actions-popover.vala @@ +48,3 @@ } else { + // Send files + section.append (_("Send File..."), "box.send_file"); Use the proper unicode ellipsis … instead of three dots.
Review of attachment 365815 [details] [review]: ::: src/Makefile.am @@ +163,3 @@ empty-boxes.vala \ tracker-iso-query.vala \ + transfer-info-row.vala \ Please also add to src/meson.build Same for transfer-popover.vala
-- GitLab Migration Automatic Message -- This bug has been migrated to GNOME's GitLab instance and has been closed from further activity. You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.gnome.org/GNOME/gnome-boxes/issues/81.