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

Crash on opening specific link #556

Closed
helpimnotdrowning opened this issue Jun 12, 2023 · 7 comments
Closed

Crash on opening specific link #556

helpimnotdrowning opened this issue Jun 12, 2023 · 7 comments
Labels
5 - high priority This issue affects core functionality and must be fixed ASAP bug Something isn't working

Comments

@helpimnotdrowning
Copy link

helpimnotdrowning commented Jun 12, 2023

** Jerboa Version **
0.0.33

Describe the bug
In this post to c/Anime@lemmy.ml : https://feddit.de/post/761086 , clicking on the markdown-styled link in the first paragraph to c/japanesemusic@feddit.de crashes the app.

I haven't seen any other links that also do this, but I will update if I find anything.

To Reproduce
Steps to reproduce the behavior:

  1. Go to https://feddit.de/post/761086
  2. Click on the link in the 1st paragraph
  3. App crashes

Using:

  • LG v60 (root)
  • Android 11
  • Jerboa 0.0.33 via Play Store
  • My user host: lemmy.sdf.org
@awdsns
Copy link

awdsns commented Jun 12, 2023

Woohoo I broke the app!

So this link is a relative one (no protocol or host part as the target, only /c/japanesemusic@feddit.de) which AFAIK is currently the only way to do sort-of portable links to communities. At least this works with the web UI.

FWIW for me Jerboa opens Chrome with the link, which then obviously doesn't work because of the missing host.

In general it would be nice if Jerboa (and other frontends) could just natively convert !community@instance into links by itself, then such hacks wouldn't be necessary.

@awdsns
Copy link

awdsns commented Jun 12, 2023

Related: #454

@awdsns
Copy link

awdsns commented Jun 12, 2023

Nevermind, for me it crashes too with 0.0.33 from Play Store. I think it was with a previous version that those links opened in Chrome.

@Anna-log7 might this be related to #413?

I think the null check on instance fails because the link doesn't in fact contain a target instance. Should it maybe default to the current instance in that case?

@Anna-log7
Copy link
Contributor

At first glance that link looks formatted differently from what I tested on (instance/c/name@instance vs instance/c/name) so I'd assume it's creating a community name of name@instance@instance

@awdsns
Copy link

awdsns commented Jun 13, 2023

But would that community name name@instance@instance cause the app to crash?

I can't install Android Studio to debug it right now and zero experience with Jetpack Compose, but if you have a link target of only /c/somecommunity@someinstance, could that still match on the uriPattern = "{instance}/c/{name}" and cause the null check on instance to blow up?

https://github.com/dessalines/jerboa/blob/e4923c9308ba618ef80a0aa31c5fb8c411d9987d/app/src/main/java/com/jerboa/MainActivity.kt#L207

beatgammit added a commit to beatgammit/jerboa that referenced this issue Jun 13, 2023
This is a partial fix to LemmyNet#556, but it at least prevents crashing when an unknown
URL type is encountered. There is still some work to do, since the
markdown plugin interprets the following as mailto: urls:

- @user@instance
- !community@instance

I'm not sure what the supported URL types are intended to be, so I
didn't bother implementing a plugin.
beatgammit added a commit to beatgammit/jerboa that referenced this issue Jun 13, 2023
This is a partial fix to LemmyNet#556, but it at least prevents crashing when an unknown
URL type is encountered. There is still some work to do, since the
markdown plugin interprets the following as mailto: urls:

- @user@instance
- !community@instance

I'm not sure what the supported URL types are intended to be, so I
didn't bother implementing a plugin.
@twizmwazin twizmwazin added bug Something isn't working 5 - high priority This issue affects core functionality and must be fixed ASAP labels Jun 13, 2023
beatgammit added a commit to beatgammit/jerboa that referenced this issue Jun 14, 2023
This is a partial fix to LemmyNet#556, but it at least prevents crashing when an unknown
URL type is encountered. There is still some work to do, since the
markdown plugin doesn't properly interpret URLs the way we want.

User-related parsing still doesn't work because there's work needed in
the markdown layer, but once that's done, this should just work as
intended.
beatgammit added a commit to beatgammit/jerboa that referenced this issue Jun 14, 2023
This is a partial fix to LemmyNet#556, but it at least prevents crashing when an unknown
URL type is encountered. There is still some work to do, since the
markdown plugin doesn't properly interpret URLs the way we want.

User-related parsing still doesn't work because there's work needed in
the markdown layer, but once that's done, this should just work as
intended.
beatgammit added a commit to beatgammit/jerboa that referenced this issue Jun 14, 2023
This is a partial fix to LemmyNet#556, but it at least prevents crashing when an unknown
URL type is encountered. There is still some work to do, since the
markdown plugin doesn't properly interpret URLs the way we want.

User-related parsing still doesn't work because there's work needed in
the markdown layer, but once that's done, this should just work as
intended.
beatgammit added a commit to beatgammit/jerboa that referenced this issue Jun 14, 2023
This is a partial fix to LemmyNet#556, but it at least prevents crashing when an unknown
URL type is encountered. There is still some work to do, since the
markdown plugin doesn't properly interpret URLs the way we want.

User-related parsing still doesn't work because there's work needed in
the markdown layer, but once that's done, this should just work as
intended.
beatgammit added a commit to beatgammit/jerboa that referenced this issue Jun 15, 2023
This is a partial fix to LemmyNet#556, but it at least prevents crashing when an unknown
URL type is encountered. There is still some work to do, since the
markdown plugin doesn't properly interpret URLs the way we want.

User-related parsing still doesn't work because there's work needed in
the markdown layer, but once that's done, this should just work as
intended.
beatgammit added a commit to beatgammit/jerboa that referenced this issue Jun 15, 2023
This is a partial fix to LemmyNet#556, but it at least prevents crashing when an unknown
URL type is encountered. There is still some work to do, since the
markdown plugin doesn't properly interpret URLs the way we want.

User-related parsing still doesn't work because there's work needed in
the markdown layer, but once that's done, this should just work as
intended.
beatgammit added a commit to beatgammit/jerboa that referenced this issue Jun 15, 2023
This is a partial fix to LemmyNet#556, but it at least prevents crashing when an unknown
URL type is encountered. There is still some work to do, since the
markdown plugin doesn't properly interpret URLs the way we want.

User-related parsing still doesn't work because there's work needed in
the markdown layer, but once that's done, this should just work as
intended.
beatgammit added a commit to beatgammit/jerboa that referenced this issue Jun 15, 2023
This is a partial fix to LemmyNet#556, but it at least prevents crashing when an unknown
URL type is encountered. There is still some work to do, since the
markdown plugin doesn't properly interpret URLs the way we want.

User-related parsing still doesn't work because there's work needed in
the markdown layer, but once that's done, this should just work as
intended.
twizmwazin pushed a commit that referenced this issue Jun 15, 2023
* Handle more URL types

This is a partial fix to #556, but it at least prevents crashing when an unknown
URL type is encountered. There is still some work to do, since the
markdown plugin doesn't properly interpret URLs the way we want.

User-related parsing still doesn't work because there's work needed in
the markdown layer, but once that's done, this should just work as
intended.

* Support http links
Chris-Kropp pushed a commit to Chris-Kropp/jerboa that referenced this issue Jun 16, 2023
* Handle more URL types

This is a partial fix to LemmyNet#556, but it at least prevents crashing when an unknown
URL type is encountered. There is still some work to do, since the
markdown plugin doesn't properly interpret URLs the way we want.

User-related parsing still doesn't work because there's work needed in
the markdown layer, but once that's done, this should just work as
intended.

* Support http links
@beatgammit
Copy link
Contributor

Ok, this should behave a lot better now, please retest.

@twizmwazin
Copy link
Contributor

Given the lack of updated info and the number of changes that have happened surrounding link handling, I'm going to mark this one as complete.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
5 - high priority This issue affects core functionality and must be fixed ASAP bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants