-
-
Notifications
You must be signed in to change notification settings - Fork 247
feat(Boost For Reddit): Add Fix /s/ links
patch
#631
Conversation
I really hate the database query part of this - however I couldn't find anything inside context that includes access token or any sort of HTTP client - so its the best solution I managed to come up with :( My network is censored weirdly - I cant open reddit without VPN, and reddit blocks VPNs |
app/src/main/java/app/revanced/integrations/boostforreddit/FixSLinksPatch.java
Outdated
Show resolved
Hide resolved
This comment was marked as resolved.
This comment was marked as resolved.
import java.net.URL; | ||
|
||
public final class FixSLinksPatch { | ||
public static String getUserAccessToken(Context context) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should be private
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should it still be private? Interfaces don't allow private+default
app/src/main/java/app/revanced/integrations/boostforreddit/FixSLinksPatch.java
Outdated
Show resolved
Hide resolved
app/src/main/java/app/revanced/integrations/boostforreddit/FixSLinksPatch.java
Outdated
Show resolved
Hide resolved
app/src/main/java/app/revanced/integrations/boostforreddit/FixSLinksPatch.java
Outdated
Show resolved
Hide resolved
This is related to s-links - to get slink reliably (incl. banned networks) Authorization header with user token must be supplied, else reddit gives 403 without proper Location. |
Do I add Sync for reddit switch to interface in this PR or should I make a separate one after that one gets merged? |
On note of implementing support for Sync for reddit: Sync doesn't store access tokens in its DB (unlike boost) - only refresh tokens. The access token in sync seem to be kept inside com.laurencedawson.reddit_sync.singleton.AccountSingleton - but I have no idea how one can even access it from integration. Unless the patch would also patch the AccountSingleton method that sets the Authorization header and somehow transfer between these two. |
Tested on Boost 1.12.12 and Sync v23.06.30-13:39. Resolution no longer halts the UI. Everything hopefully should be good, gonna run around with boost for a few days and check if everything gonna be okay. |
Fix /s/ links
patch
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The abstraction can be improved. Theres still some duplicate implementations such as openInAppBrowser or resolveSLink. Instead of copying the implementation, try to reduce duplicates. In the example of openInAppBrowser you could for example add a field of type Activity and then set the field in the constructor of the instance with either activity for Sync and Boost, then the base class implementation could reference it. You would then not need to duplicate the implementation like you do now in the implementing classes.
Are you suggesting that both Boost & Sync use the same class, and inside constructor there is some sort of switch-case that determines webview activity based on package name? |
app/src/main/java/app/revanced/integrations/boostforreddit/FixSLinksPatch.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Left some refactoring changes and TODOs in the code. Please check on them and test the refactoring changes (same for the patches part). Apart from that lgtm.
app/src/main/java/app/revanced/integrations/shared/fixes/slink/BaseFixSLinksPatch.java
Outdated
Show resolved
Hide resolved
I am not sure on todo you added:
Wouldnt that be caught by catch (Exception e)? |
You originally left a comment claiming that Objects.requireNonNull was causing a RuntimeException, but given that you were not able to reproduce again the question is redundant. |
Thank you for contributing to ReVanced. Join us on Discord to receive a role for your contribution. |
# [1.11.0-dev.3](v1.11.0-dev.2...v1.11.0-dev.3) (2024-06-08) ### Features * **Boost For Reddit:** Add `Fix /s/ links` patch ([#631](#631)) ([0c9ad35](0c9ad35))
# [1.11.0](v1.10.0...v1.11.0) (2024-06-23) ### Bug Fixes * **YouTube - Client spoof:** Correctly play more livestreams using Android VR ([#652](#652)) ([58f8172](58f8172)) * **YouTube - Hide description components:** Replace `Hide game section` and `Hide music section` with `Hide attributes section` ([#654](#654)) ([f82dfce](f82dfce)) * **YouTube - Return YouTube Dislike:** Do not replace view count with dislikes ([5f79196](5f79196)) * **YouTube - Spoof client:** Correctly play as background audio only with Android VR ([9adbc66](9adbc66)) * **YouTube - Spoof client:** Correctly play some live streams when using Android VR ([f74fb17](f74fb17)) * **YouTube:** Rename `Minimized playback` to `Remove background playback restrictions` ([#651](#651)) ([84c50c0](84c50c0)) ### Features * **Boost For Reddit:** Add `Fix /s/ links` patch ([#631](#631)) ([0c9ad35](0c9ad35)) * **YouTube - Comments:** Add `Hide 'Create a Short' button` option ([#656](#656)) ([064d8e9](064d8e9)) * **YouTube - Comments:** Add `Hide Thanks button` and `Hide 'Comments by members' header` options ([#653](#653)) ([240e805](240e805)) * **YouTube - Miniplayer:** Rename `Tablet mini player` and allow selecting the style of the in-app miniplayer ([#649](#649)) ([f483af6](f483af6))
Multi-part PR:
Fix /s/ links
patch revanced-patches#3154This PR fixes ReVanced/revanced-patches#478
Code is essentially copy-paste from patch for Sync, but with more exception handling and adding the Authorization header to prevent reddit from giving the client page "log in or provide token or else" without proper Location: header.