fix: lnurl scheme parsing #2975
Conversation
ekzyis
force-pushed
the
2927-fix-lnurl-scheme-parsing
branch
from
ffcdb6b to
1c63e51
Compare
|
had a round of tests. LGTM cc @rolznz the error is just because we use more generalised matcher which can cause such error with relative links having lnurl inlcuded in the link |
| @@ -45,10 +45,12 @@ function registerLightningLinkClickHandler() { | |||
| return; | |||
| } | |||
| const lightningLink = target.closest('[href^="lightning:" i]'); | |||
| const lnurlLink = target.closest('[href^="lnurl" i]'); | |||
| const lnurlLink = target.closest( | |||
There was a problem hiding this comment.
Could we link to the relevant spec? I think it is LUD-17
There was a problem hiding this comment.
Should we support keyauth too? (I think this change will currently break it)
There was a problem hiding this comment.
Could we link to the relevant spec? I think it is LUD-17
Done in 385af36
Should we support keyauth too? (I think this change will currently break it)
I think you never supported keyauth so this change will not break it. I searched for keyauth in your source code and no occurrence was found:
$ grep -R "keyauth" src/
I have never seen keyauth in the wild and LNURL-auth already works with Alby. So I think this is a part of a spec that isn't necessary to implement. But might still make sense to add it.
There was a problem hiding this comment.
Ah ok, thanks! I'll made a new issue for that: #2984
Describe the changes you have made in this PR
In #2927, I noticed that links that start with
lnurlopen the Alby extension. This means that even links to content unrelated to lightning likelnurlpay_comments.mp4open the Alby extension.I looked up what schemes are meant to be parsed and I found LUD-17.
So I think only the following link prefixes should open the Alby extension:
lightning:,lnurlc:,lnurlw:,lnurlp:Link this PR to an issue [optional]
Fixes #2927
Type of change
fix: Bug fix (non-breaking change which fixes an issue)Screenshots of the changes [optional]
video
2023-12-31.01-07-47.mp4
How has this been tested?
Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration
Checklist