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

Fix intent filters #247

Merged
merged 8 commits into from
Mar 28, 2024
Merged

Fix intent filters #247

merged 8 commits into from
Mar 28, 2024

Conversation

ArnyminerZ
Copy link
Member

@ArnyminerZ ArnyminerZ commented Mar 14, 2024

Working features:

  • Opening a webcal:// or webcals:// link with the app.
  • Opening an *.ics file with the app.
  • Sharing a link with the app.
  • Sharing an *.ics file with the app.

Remaining bugs:

  • Launching a file from the file explorer opens ICSx5, and adds the URL to the field, but doesn't update showNextButton, or at least it's not updated on the UI, so the next step cannot be reached. If a character is removed and added back to the url field, the button shows up.
  • file:// urls do not work. However, I'm not sure if they should be used, or we should stick to content://

Signed-off-by: Arnau Mora Gras <[email protected]>
Signed-off-by: Arnau Mora Gras <[email protected]>
Signed-off-by: Arnau Mora Gras <[email protected]>
@ArnyminerZ ArnyminerZ self-assigned this Mar 14, 2024
@ArnyminerZ ArnyminerZ linked an issue Mar 14, 2024 that may be closed by this pull request
3 tasks
@ArnyminerZ ArnyminerZ marked this pull request as ready for review March 14, 2024 17:03
@ArnyminerZ ArnyminerZ requested a review from sunkup March 14, 2024 17:03
Copy link
Member

@sunkup sunkup left a comment

Choose a reason for hiding this comment

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

file:// urls do not work. However, I'm not sure if they should be used, or we should stick to content://

Not even on Android 6 (Level 23)? If that is the case we should remove them from the manifest.

Also I don't understand the answer you gave in the issue on HTTPS URLs do they work, or not? With the current state of the PR, I can not add it from the browser. The browser will just start a download. Where did you find that wildcards are prohibited or we need verified domains for it?

If it does not work we should remove http(s) from the intent filter in the manifest too :)

@ArnyminerZ
Copy link
Member Author

Not even on Android 6 (Level 23)?

It was introduced in Android 7. See the changelog

Also I don't understand the answer you gave in the issue on HTTPS URLs do they work, or not?

No

With the current state of the PR, I can not add it from the browser. The browser will just start a download.

Yes, that's the thing, only the browser is authorized to open "unknown" domains.

Where did you find that wildcards are prohibited or we need verified domains for it?

Technically we are using them as App Links, since our app is not a web browser. For that, we need to either "Add intent filters that contain the autoVerify attribute." or "Declare the association between your website and your intent filters by hosting a Digital Asset Links JSON file [...]".

In both cases, we need to be the owner of the website we are trying to open to verify our app. It wouldn't be crazy to add automatic verification for Nextcloud, for example, by adding our verification file to the repo, which would be installed on all instances. I don't know if they would want that, of course, and it's still not a solution needing to have it added everywhere. Also we would have to keep a list of the verified domains on the app, which is unsustainable.

What we can actually try to add is supporting share intent filters. For example, as shown here. That way users can add the link by sharing it to the app. It's not as convenient as just opening the link, but it's a good solution I think.

@sunkup
Copy link
Member

sunkup commented Mar 21, 2024

Not even on Android 6 (Level 23)?

Ok, so then let's leave the file:// URLs for now, as you said.

Also I don't understand the answer you gave in the issue on HTTPS URLs do they work, or not?

No

With the current state of the PR, I can not add it from the browser. The browser will just start a download.

Yes, that's the thing, only the browser is authorized to open "unknown" domains.

Then we should remove this

  <data android:pathPattern=".*\\.ics" />
  <data android:pathPattern=".*\\..*\\.ics" />
  <data android:pathPattern=".*\\..*\\..*\\.ics" />
  <data android:pathPattern=".*\\..*\\..*\\..*\\.ics" />

from the manifest, no?

What we can actually try to add is supporting share intent filters. For example, as shown here. That way users can add the link by sharing it to the app. It's not as convenient as just opening the link, but it's a good solution I think.

Sounds good, in theory browsers and other apps should be able to share "text/calendar" mime type files to ICSx5 then. Even if Chrome or firefox can't do it now, they might in the future. I think should add this then :)

@ArnyminerZ
Copy link
Member Author

Then we should remove this [...] from the manifest, no?

Actually I'm not sure if Android detects the mime type correctly. If it does, yes, we should remove that. If not, we have to leave it. I'll test that one to be sure.

I think should add this then :)

I'll add it 😄

@ArnyminerZ ArnyminerZ requested a review from sunkup March 25, 2024 13:00
Copy link
Member

@sunkup sunkup left a comment

Choose a reason for hiding this comment

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

Nice! How do you test webcal(s):// links though? If I open webcals://cdn.arnyminerz.com/sample.ics in firefox, it opens ICSx5 (yay!), but without the actual URL - see the picture below :)

image

@ArnyminerZ
Copy link
Member Author

Mmmh should work fine. I'll check it again

Signed-off-by: Arnau Mora Gras <[email protected]>
@ArnyminerZ
Copy link
Member Author

Done. Can you check again?

@sunkup sunkup self-requested a review March 28, 2024 10:55
Copy link
Member

@sunkup sunkup left a comment

Choose a reason for hiding this comment

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

Yup, works fine now

@sunkup sunkup merged commit a77733b into dev Mar 28, 2024
7 checks passed
@sunkup sunkup deleted the 246-possibly-broken-intent-filters branch March 28, 2024 10:56
@sunkup sunkup modified the milestones: 2.1.1, 2.2.2 Apr 15, 2024
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.

Possibly broken intent filters
2 participants