Skip to content

Some improvements#540

Merged
crimera merged 6 commits intocrimera:devfrom
kitadai31:kitadai31-patch-0306
Mar 17, 2025
Merged

Some improvements#540
crimera merged 6 commits intocrimera:devfrom
kitadai31:kitadai31-patch-0306

Conversation

@kitadai31
Copy link
Contributor

@kitadai31 kitadai31 commented Mar 9, 2025

Hi 👋
I made some improvements of piko patches.
This pull request contains 6 commits.

Integrations: crimera/revanced-integrations#121

1. fix(Bring back twitter): Resource compilation fails on ReVanced Manager

I also simplified some constants of this patch.

2. fix(Enable Reader Mode): Specify the compatible version, show a warning instead of throwing an exception when failed

Compatible version is useful for users who want to use reader mode.
Also, if the patch has failed, show a warning instead of showing a wall of the stack trace.

WARNING: "Enable Reader Mode" is not supported in this version. Use X 10.72.2 or earlier. The patch was not executed.

Screenshot

[ADDED]: This change add a notice of compatible version only to the patch description. Because if I add a target version to the patch, Manager will no longer show "Any" for suggested version.

3. docs: Add compatible CLI and Manager versions to README until bumping to ReVanced Patcher v21

This is absolutely necessary!

4. fix(Remove Detailed posts): Change the settings label from "detailed" to "related"

"Tweet Detail Related Tweets" was mistakenly abbreviated to "Detailed".
"Related" is more accurate word to describe "Discover more" section.

Note: The name of the patch is not changed in this commit.
Because renaming the patch name might affect in manager/builder that's use this patch name.
Variable names and string keys are also not changed as per the patch name.
Only the setting label has been changed.

I also fixed a typo "Revist" as well.

5. fix: Remove non-functional Open browser chooser on opening links patch

The last working version of this patch was 10.48.0-release.0.
So, I tried updating this patch according to #442.
Then, I fixed the patch and I checked that the integrations method (openWithChooser) is called correctly when I open any links. (Checked with logs)

However, the app chooser still doesn't open.
It seems that Intent.createChooser() has no effect.
Tested on Pixel 3, Android 14 (LineageOS 21).
3 browsers are installed: Chrome, Firefox and Via.

So I think this patch is not working and should be removed.

I also tested the link which has an external app.
For example, I installed Pixiv app on my phone, so when I opened a Pixiv link on patched Twitter, the app chooser should be opened and show "Chrome or Firefox or Via or Pixiv?" choices.
But it opens the Pixiv app directly. App chooser wasn't opened.

For reference, these are the patch source I used.
https://github.com/kitadai31/piko/blob/5d1e295c41f9b98299e1de820bd7d7ba5d90de7a/src/main/kotlin/crimera/patches/twitter/link/browserchooser/BrowserChooserPatch.kt
https://github.com/kitadai31/piko/blob/5d1e295c41f9b98299e1de820bd7d7ba5d90de7a/src/main/kotlin/crimera/patches/twitter/link/browserchooser/fingerprints/OpenLinkFingerprint.kt

@dic1911
Hello, if you are reading this, can you please let me know if you actually managed to open the app chooser by #442 method?
And what Android version are you using?
I suspect this has to do with a change in Android 12.

6. feat(Settings): Add a description to "Native features" page

The usage of native features were unclear.
This helps users find the native features.

Screenshot

I have one question for this commit.

I changed "detailed" to "related" only for English strings.
I checked with Google Translate and found that it is still translated as "detailed post" in some other languages.
Should I delete the changed string from all languages?
Or shouldn't?

@dic1911
Copy link
Contributor

dic1911 commented Mar 9, 2025

Hello, if you are reading this, can you please let me know if you actually managed to open the app chooser by #442 method?
And what Android version are you using?
I suspect this has to do with a change in Android 12

Hi, thanks for trying to help, I was on Android 14 back then and now on 15, the provided patch is working on both Android version for Twitter 10.63 (Sorry but I don't really update modded apks unless something breaks so I couldn't verify if the same applies to newer updates)

edit: @kitadai31 btw if you want I can share my apk so you can easily test it

@kitadai31
Copy link
Contributor Author

Oh, really? Hmm...
Then it may be a target Twitter version problem? I patched 10.82.0.

edit: @ kitadai31 btw if you want I can share my apk so you can easily test it

Thank you 🙏
I want to test with it
If you have a Telegram or Discord, please send to @ kitadai31

@crimera
Copy link
Owner

crimera commented Mar 10, 2025

These patches are much appreciated. Thank you

Should I delete the changed string from all languages?
Or shouldn't?

let's just keep them.

@dic1911
Copy link
Contributor

dic1911 commented Mar 10, 2025

@kitadai31 since we're here, can you also send me your patched APK to check if the smali instructions were patched properly?

Edit: Just for the record, I had my APK sent to @kitadai31 via Telegram earlier

@kitadai31
Copy link
Contributor Author

@dic1911 I tried your manually modified 10.63.1 apk, but the app chooser still don't open.

However, just only once, when I tapped on an external link on a profile page, the app chooser appeared, but it said "There is no app for this action."
After that, the app chooser never appeared again. I can't reproduce it. 😕

OK, I'll send my patched apk tomorrow

@dic1911
Copy link
Contributor

dic1911 commented Mar 11, 2025

@dic1911 I tried your manually modified 10.63.1 apk, but the app chooser still don't open.

However, just only once, when I tapped on an external link on a profile page, the app chooser appeared, but it said "There is no app for this action." After that, the app chooser never appeared again. I can't reproduce it. 😕

@kitadai31 This sound like what would happen when you have default browser set, can you try to freeze the browser which it opens automatically when you tap on links?

@kitadai31
Copy link
Contributor Author

@dic1911

can you try to freeze the browser which it opens automatically when you tap on links?

It works!
After I unset the default browser by uninstalling it, the browser chooser started to appear.
Working on both your 10.63 apk and my patched 10.82 apk.

Screenshot_20250311-142600_IntentResolver

I thought that patch allows us to choose browser every time even if the default browser is set.
It might be better to add a note about it in the description of the "Use browser chooser" setting.

@kitadai31
Copy link
Contributor Author

kitadai31 commented Mar 11, 2025

But, if the default browser is not set, the stock Twitter also opens a normal intent chooser.
(You need to turn off X settings > Display > "Use in-app browser")

Screenshot_20250311-163649_Android System

This dialog is a little different from the app chooser in that it has an "Always" button, but I think it is almost same. (to be honest)
So, is this patch necessary...?

@dic1911
Copy link
Contributor

dic1911 commented Mar 12, 2025

@kitadai31

even if the default browser is set.

It's just how Android works, AFAIK when a default browser is set and you want to open the link with a chooser for something else, you need to iterate through apps to check what apps can open the link and make UI yourself

@kitadai31
Copy link
Contributor Author

@dic1911 How about this? #540 (comment)
I found that the stock Twitter also opens browser chooser if the default browser is not set.
Therefore, this patch may no longer be needed.

It is possible that the behavior of the Twitter app has changed in recent versions after you contributed the browser chooser patch.

But if you think that patch is still useful, of course I'll drop the commit d654ba4 and fix the patch instead.

@dic1911
Copy link
Contributor

dic1911 commented Mar 14, 2025

@kitadai31 Since I don't really know when it's changed, maybe just leave the patch there and add a note so people would know it should only be applied on older versions?

@kitadai31
Copy link
Contributor Author

It is possible that the behavior of the Twitter app has changed in recent versions after you contributed the browser chooser patch.

↑ To see if it is correct, I just tried Twitter 10.23.0.
As a result, this version already opens the browser selector if no default browser is set.
Tested on Android 7.1 and 14.
So this was not a change in recent versions.
Maybe it was an oversight.

Since I don't really know when it's changed, maybe just leave the patch there and add a note so people would know it should only be applied on older versions?

Based on the above result, I don't think that's necessary.
10.23.0 is a sufficiently old version.
And it's too old to apply the piko patch.
(None of the patches are available for 10.23.0 because the build fails due to Settings patch.)
Therefore, the browser selector should appear even on the oldest Twitter that can apply the piko patch.

@dic1911
For these reasons, may I remove the patch?

Alternatively, I can fix the fingerprint so that the patch fails successfully in 10.49+.
The current fingerprint incorrectly matches the Integrations method openWithChooser, which causes the patch to succeed on 10.49+ accidentally.

@dic1911
Copy link
Contributor

dic1911 commented Mar 15, 2025

@kitadai31 I'd ask @crimera for opinion instead since I don't really own piko ;)

@kitadai31 kitadai31 marked this pull request as draft March 15, 2025 10:12
…ng instead of throwing an exception when failed
… to "related"

"Tweet Detail Related Tweets" was mistakenly abbreviated to "Detailed".
"Related" is more accurate word to describe "Discover more" section.

However, the name of the patch is not changed in this commit.
Because renaming the patch name might affect in manager/builder that's use this patch name.
Variable names and string keys are also not changed as per the patch name.
The last working version was 10.48.0.
Moreover, it is possible on the stock Twitter.
@kitadai31 kitadai31 force-pushed the kitadai31-patch-0306 branch from 9046295 to be7aa54 Compare March 15, 2025 19:09
@kitadai31
Copy link
Contributor Author

kitadai31 commented Mar 15, 2025

I'd ask crimera for opinion instead since I don't really own piko ;)

Fair enough.

I re-pushed the PR branch with some updates:

  • Simplified the Manager check in Bring back twitter patch
  • Changed the commit message for the browser chooser patch commit

@crimera Ready for review!

@kitadai31 kitadai31 marked this pull request as ready for review March 15, 2025 19:19
@crimera crimera merged commit 4241c7a into crimera:dev Mar 17, 2025
1 check passed
@kitadai31 kitadai31 deleted the kitadai31-patch-0306 branch March 18, 2025 03:29
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.

3 participants