Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[feature/url-shortcuts] Open and create .url shortcut files #1344

Merged
merged 12 commits into from May 13, 2024

Conversation

felix-schwarz
Copy link
Contributor

@felix-schwarz felix-schwarz commented Apr 11, 2024

Description

Adds the ability to open shortcuts stored in .url files as well as to create them.

The MDM option action.open-shortcut-mode provides control over what kind of shortcuts (URLs, items) should be allowed to be opened. Defaults to all.

Screenshots (if appropriate):

Action in empty folder and on + button Creating a shortcut Shortcut in file list
Simulator Screenshot - iPad Air (5th generation) - 2024-04-17 at 15 34 12 Simulator Screenshot - iPad Air (5th generation) - 2024-04-17 at 15 30 20 Simulator Screenshot - iPad Air (5th generation) - 2024-04-17 at 15 33 20
Create file shortcut Create URL shortcut Opening a URL shortcut
Simulator Screenshot - iPad Air (5th generation) - 2024-04-17 at 15 31 13
Simulator Screenshot - iPad Air (5th generation) - 2024-04-17 at 15 31 17
Simulator Screenshot - iPad Air (5th generation) - 2024-04-17 at 15 33 14 Simulator Screenshot - iPad Air (5th generation) - 2024-04-17 at 15 33 25

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

@CLAassistant
Copy link

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

@felix-schwarz felix-schwarz self-assigned this Apr 11, 2024
@felix-schwarz felix-schwarz added this to the 12.2-Next milestone Apr 11, 2024
Copy link
Collaborator

@hosy hosy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would suggest to remove the URLDisplayViewController. Instead I would choose one of the following approaches:

  • show an UIAlertViewController with question "Open Link?", on "Yes" it opens the link in the system browser
  • open the link directly In-App in a SafariViewController

The detail view does not give any further content to the user and can be removed in my opinion.

@felix-schwarz felix-schwarz requested a review from hosy April 17, 2024 15:23
@felix-schwarz
Copy link
Contributor Author

@hosy Thanks for the feedback. The improved implementation now allows opening shortcut files directly from the file list and brings up an alert asking the user whether the URL should be opened:
Simulator Screenshot - iPad Air (5th generation) - 2024-04-17 at 15 33 25

I still kept the viewer for shortcut files around for those cases where a user views files in the same folder and eventually reaches a shortcut file when swiping through.

@jesmrec
Copy link
Contributor

jesmrec commented Apr 17, 2024

I did a first look into the feature (before QA - CR), with a couple of comments on my side

  • Shortcuts to url show the following question: Should it be opened in the default browser? Well, this question is not really a question since there is no other choice than opening in the default browser (cancel is not a real option to open). So, i'd change that for an statement like It will be opened in the default browser to clarify what's going to happen. More complex options like letting user select another browser to open are not worthy IMO (future improvement¿?). But, questions to users should have at least two real choices. Also, removing the question is not a bad option... the easier the better. Just as a suggestion.

  • As improvement: if the target URL does not exist (for example, a link to a file or folder that has been removed from account), the message is Operation couldn't be completed. It would be cool a more accurate message like file does not exist. Private links show: item is not known to the server as another example

Will do deeper testing after CR but it really looks good at this point.

@felix-schwarz
Copy link
Contributor Author

@jesmrec Thanks for the feedback. I've trimmed down the alert asking for opening the link and removed the browser reference entirely. Also, the error message when the target of a shortcut can't be found is now on point and informative (or at least I hope so 😉).

I also added a action.open-shortcut-mode MDM option that allows admins to control which type of shortcut content they want to allow (all, URLs only, items only, none).

Copy link
Collaborator

@hosy hosy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome feature and implementation!

@jesmrec
Copy link
Contributor

jesmrec commented May 2, 2024

QA checks:

  • Option in +
  • URL or name empty
  • URL of web
  • URL of file in oC same account and space
  • URL of file in oC same account and different space
  • URL of file in oC different account [NA]
  • Incorrect URL (points nowhere)
  • URL pointing to removed file

@jesmrec
Copy link
Contributor

jesmrec commented May 2, 2024

(1)

In oC10 server:

  1. Create shortcut
  2. Add as URL a web like https://owncloud.com/
  3. Create shortcut
  4. Click on the shortcut on list of file

Current: not opened, also the thumbnail in file list is not correct

NOTE: same behaviour when files are shortcuted.

iPhoneXR iOS17
f1b6e0e6

@jesmrec
Copy link
Contributor

jesmrec commented May 3, 2024

(2)

As a question: shortcuts over files are only allowed over items in the same account, right? not posible to create a shortcut that points to a file in other account attached to the device (moving to there).

@felix-schwarz
Copy link
Contributor Author

felix-schwarz commented May 3, 2024

@jesmrec Regarding (1):

The current implementation supporting .url files depends on the MIME type text/uri-list, which OC10 appears not to use for .url-files. Instead it returns application/octet-stream:

<?xml version="1.0"?>
<d:multistatus xmlns:d="DAV:" xmlns:s="http://sabredav.org/ns" xmlns:oc="http://owncloud.org/ns">
  <d:response>
    <d:href>/remote.php/dav/files/admin/Files.url</d:href>
    <d:propstat>
      <d:prop>
        <d:resourcetype/>
        <d:getlastmodified>Fri, 03 May 2024 15:48:47 GMT</d:getlastmodified>
        <d:getcontentlength>51</d:getcontentlength>
        <d:getcontenttype>application/octet-stream</d:getcontenttype>
        <d:getetag>"ba2042d735ee8e0bd77c95655aedb8e6"</d:getetag>
        <oc:id>00009374oc60lgadtioc</oc:id>
        <oc:size>51</oc:size>
        <oc:permissions>RDNVW</oc:permissions>
        <oc:favorite>0</oc:favorite>
        <oc:share-types/>
        <oc:owner-id>admin</oc:owner-id>
        <oc:owner-display-name>admin</oc:owner-display-name>
        <oc:checksums>
          <oc:checksum>SHA1:9405c1b721fdd574d50d639dc14983f8906f45fd MD5:cdc846c7f7b2980ca79b452fb3015f9b ADLER32:f32e1229</oc:checksum>
        </oc:checksums>
      </d:prop>
      <d:status>HTTP/1.1 200 OK</d:status>
    </d:propstat>
  </d:response>
</d:multistatus>

Once OC10 returns text/uri-list for .url-files, handling in the iOS app should work as expected.

Regarding (2):
There's currently no support for cross-server shortcuts.

@jesmrec
Copy link
Contributor

jesmrec commented May 6, 2024

That's approved on my side.

@jesmrec
Copy link
Contributor

jesmrec commented May 6, 2024

Before merging, the branch should point to a different base branch like milestone/12.3

@felix-schwarz felix-schwarz changed the base branch from milestone/12.2 to milestone/12.3 May 7, 2024 08:44
felix-schwarz and others added 10 commits May 13, 2024 10:12
- INIFile+URLFile: simple extension for quickly parsing and composing .url files
- URLDisplayViewController: stub implementation to handle .url files
…ng shortcut URL

- CreateURLShortcutAction: new action to create a URL shortcut in a folder
- design and add "text-uri-list" icon
… open files from file lists, overriding viewers if needed

- OCItem+Interactions: add support for `.directOpen` actions
- INIFile+ShortcutResolution: implement support for shortcuts to other items on the same server
- ShortcutFileDisplayViewController: implement support for shortcuts to other items on the same server, including an item preview
- add new action OpenShortcutFileAction
	- hooks into the `.directOpen` location
	- uses INIFile+ShortcutResolution to open the contained link or jump to the targeted location
	- asks users before opening links
- rename files for better distinctibility:
	- URLDisplayViewController -> ShortcutFileDisplayViewController
	- CreateURLShortcutAction -> CreateShortcutFileAction
- OCItem+UniversalItemListCellContentProvider: fix warning
- CreateShortcutFileAction: break out creation functionality into its own view controller
- CreateShortcutFileViewController:
	- dedicated view controller for creating shortcuts
	- allows picking files and folders for shortcuts to point to and presents an embedded preview for them
	- improved error handling
- INIFile: fix formatting bug that could accidentally leak memory into the .url file
- ClientLocationPicker: add ability to allow picking files via new .allowFileSelection property
…targeting unavailable/detached drives and removes the respective policies

- adapt app code to SDK API changes
	- improved error reporting for when the target of a shortcut can't be accessed / found
	- simplified and generalized prompt for opening the URL a shortcut points to
…ShortcutResolution: add MDM option to allow all shortcuts, only URL shortcuts, only item shortcuts or no shortcuts
…root, the name for the shortcut was not derived correctly
- Localizable.strings: re-add strings auto-removed by git during rebase
@felix-schwarz felix-schwarz changed the base branch from milestone/12.3 to master May 13, 2024 08:22
@felix-schwarz felix-schwarz changed the base branch from master to milestone/12.3 May 13, 2024 08:22
@felix-schwarz felix-schwarz merged commit 1862e4f into milestone/12.3 May 13, 2024
2 of 3 checks passed
@delete-merged-branch delete-merged-branch bot deleted the feature/url-shortcuts branch May 13, 2024 08:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants