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

Replace HTML with native text formatting #1256

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

meenbeese
Copy link
Contributor

This PR removes the backwards-incompatible text formatting done with HTML throughout the codebase and replaces it with a more native approach that uses a mix of text and graphics APIs like SpannableString, Typeface, etc.

Why This is Better

  • Removes Html.fromHtml(): No deprecated HTML parsing.
  • Native Android Approach: Uses SpannableString.
  • Improved Readability & Customization: Easy to add more formatting.

Closes #1229

Tested on Pixel 8 API 34 Emulator.

@syphyr
Copy link
Contributor

syphyr commented Feb 28, 2025

error: unable to create symlink app/src/main/assets/LICENSE: File name too long
fatal: cherry-pick failed

@meenbeese
Copy link
Contributor Author

error: unable to create symlink app/src/main/assets/LICENSE: File name too long fatal: cherry-pick failed

Where are you getting this issue? On the contrary, this PR does not symlink to LICENSE anymore.

@syphyr
Copy link
Contributor

syphyr commented Feb 28, 2025

error: unable to create symlink app/src/main/assets/LICENSE: File name too long fatal: cherry-pick failed

Where are you getting this issue? On the contrary, this PR does not symlink to LICENSE anymore.

just doing a normal cherry pick, I get that error.

git fetch https://github.com/meenbeese/orbot sdk-21-fix && git cherry-pick aa74b74

@meenbeese
Copy link
Contributor Author

It is working fine on my machine. Why do you have to cherry-pick? Can't you just checkout the branch?

@syphyr
Copy link
Contributor

syphyr commented Feb 28, 2025

It is working fine on my machine. Why do you have to cherry-pick? Can't you just checkout the branch?

I'm really not sure what the problem is. I've never seen this error before when cherry picking. Maybe its a bug in m version of git.

@syphyr
Copy link
Contributor

syphyr commented Feb 28, 2025

git checkout -b html_fix meenbeese/sdk-21-fix
error: unable to create symlink app/src/main/assets/LICENSE: File name too long
D app/src/main/assets/LICENSE
Branch 'html_fix' set up to track remote branch 'sdk-21-fix' from 'meenbeese'.
Switched to a new branch 'html_fix

@syphyr
Copy link
Contributor

syphyr commented Feb 28, 2025

Cant checkout the branch either.

@syphyr
Copy link
Contributor

syphyr commented Feb 28, 2025

I've fixed it by manually copying the LICENSE file to app/src/main/assets/LICENSE and appending the commit.

It should look like this:

diff --git a/app/src/main/assets/LICENSE b/app/src/main/assets/LICENSE
deleted file mode 120000
index 14776154..00000000
--- a/app/src/main/assets/LICENSE
+++ /dev/null
@@ -1 +0,0 @@
-../../../../LICENSE
\ No newline at end of file
diff --git a/app/src/main/assets/LICENSE b/app/src/main/assets/LICENSE
new file mode 100644
index 00000000..ad7d58e9
--- /dev/null
+++ b/app/src/main/assets/LICENSE
@@ -0,0 +1,181 @@
+This file contains the license for Orbot, a free software project to
+provide anonymity on the Internet from a Google Android device.

@syphyr
Copy link
Contributor

syphyr commented Feb 28, 2025

The commit with the problem looks like this:

diff --git a/app/src/main/assets/LICENSE b/app/src/main/assets/LICENSE
index 14776154..ad7d58e9 120000
--- a/app/src/main/assets/LICENSE
+++ b/app/src/main/assets/LICENSE
@@ -1 +1,181 @@
-../../../../LICENSE
\ No newline at end of file
+This file contains the license for Orbot, a free software project to
+provide anonymity on the Internet from a Google Android device.
+
+It also lists the licenses for other components used by Orbot, including
+Tor.

@syphyr
Copy link
Contributor

syphyr commented Feb 28, 2025

But, is it really necessary to remove the symlink to LICENSE and copy the file there so it exists twice?

@syphyr
Copy link
Contributor

syphyr commented Feb 28, 2025

Oh, I see the problem. Your commit should just remove the symlink, but not include the copied file. build.gradle copies the LICENSE file during the build automatically.

It should look like this:

app-tv/src/main/java/org/torproject/android/tv/TeeveeMainActivity.java                                     | 27 ++++++++++++++++++++++-----
 app/build.gradle                                                                                           |  7 +++++++
 app/src/main/assets/LICENSE                                                                                |  1 -
 app/src/main/java/org/torproject/android/ConnectFragment.kt                                                | 43 ++++++++++++++++++++-----------------------
 app/src/main/java/org/torproject/android/ui/AboutDialogFragment.kt                                         | 20 ++++++++++++--------
 app/src/main/java/org/torproject/android/ui/v3onionservice/OnionServiceActionsDialogFragment.java          | 11 +++++++++--
 app/src/main/java/org/torproject/android/ui/v3onionservice/clientauth/ClientAuthActionsDialogFragment.java | 10 ++++++++--
 7 files changed, 78 insertions(+), 41 deletions(-)
--- a/app/build.gradle
+++ b/app/build.gradle
@@ -126,3 +126,10 @@ dependencies {
     androidTestImplementation(libs.androidx.junit)
     androidTestImplementation(libs.fastlane.screengrab)
 }
+
+tasks.register("copyLicenseToAssets", Copy) {
+    from("../LICENSE")
+    into("$projectDir/src/main/assets")
+}
+
+preBuild.dependsOn copyLicenseToAssets
diff --git a/app/src/main/assets/LICENSE b/app/src/main/assets/LICENSE
deleted file mode 120000
index 14776154..00000000
--- a/app/src/main/assets/LICENSE
+++ /dev/null
@@ -1 +0,0 @@
-../../../../LICENSE
\ No newline at end of file

@syphyr
Copy link
Contributor

syphyr commented Feb 28, 2025

Here is a fixed version: syphyr@3ca2798

@meenbeese
Copy link
Contributor Author

I don't understand why I shouldn't include the license file in the commit if it is going to be built anyway. Might as well save that step now.

@meenbeese
Copy link
Contributor Author

But, is it really necessary to remove the symlink to LICENSE and copy the file there so it exists twice?

Yes because symlinking doesn't work in debug mode so the about page was always broken there. Also, this change doesn't add any maintenance burden as the copying over occurs automatically on every build.

@syphyr
Copy link
Contributor

syphyr commented Mar 1, 2025

This commit should just delete the symlink to the license file because build.gradle copies it to assets during the build processes. Or, if your intention is to include another copy of the license file to put in assets by this commit, then you won't need the additional code added to build.gradle to copy the license file to assets.

The license file needs to be removed from this commit because it is copied to assets by the build processes.

@meenbeese
Copy link
Contributor Author

My intention was the former. Updated now to remove the symlink.

@syphyr
Copy link
Contributor

syphyr commented Mar 5, 2025

My intention was the former. Updated now to remove the symlink.

Thanks. Just tested it and its working fine now.

@syphyr
Copy link
Contributor

syphyr commented Mar 17, 2025

app/src/main/assets/LICENSE should also be added to .gitignore

@meenbeese
Copy link
Contributor Author

Merge @bitmold ?

@syphyr
Copy link
Contributor

syphyr commented Mar 26, 2025

The full path to the copied version of the LICENSE file should be used in .gitignore so the original version is also not ignored. Like this: app/src/main/assets/LICENSE

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.

[BUG] Regression: 17.0.0 RC 1 works, but 17.2.1 RC 2 and newer crashes immediately upon opening the app
2 participants