Feat[Multimedia]: render `svg` using webview #17035

criticalAY · 2024-09-07T19:47:20Z

Purpose / Description

Render SVG instead of showing a warning text.

Blocked by

fix: keep the format of imported images and remove 1MB limit #17014

Fixes

Fixes Display SVGs in the image import preview #17029

Approach

Use a webview to render an SVG in it

How Has This Been Tested?

Tested on OnePlus Nord CE:

Checklist

Please, go through these checks before submitting the PR.

You have a descriptive commit message with a short title (first line, max 50 chars).
You have commented your code, particularly in hard-to-understand areas
You have performed a self-review of your own code
UI changes: include screenshots of all affected screens (in particular showing any new or changed strings)
UI Changes: You have tested your change using the Google Accessibility Scanner

github-actions · 2024-09-07T19:47:33Z

Message to maintainers, this PR contains strings changes.

Before merging this PR, it is best to run the "Sync Translations" GitHub action, then make and merge a PR from the i18n_sync branch to get translations cleaned out.
Then merge this PR, and immediately do another translation PR so the huge change made by this PR's key changes are all by themselves.

Read more about updating strings on the wiki,

BrayanDSO

Looks GREAT to me

BrayanDSO · 2024-09-09T09:50:24Z

Question: does this work for other formats?

Because animated PNGs are static in the ImageView, but animated when the card is opened

criticalAY · 2024-09-10T16:38:00Z

Question: does this work for other formats?

No this method would require workarounds for different images, we can directly use webview to preview images

BrayanDSO · 2024-09-10T23:17:08Z

we can directly use webview to preview images

So, is it possible to use the WebView instead of an ImageView to preview all the images?

That should handle all the images that can be rendered in a flashcard

mikehardy

this does seem like a fantastic opportunity to have a single way to do things vs conditionals

what if all images went through the webview? can this PR be altered to do that?

also would be nice to move this along by removing the dependency blocker - I think that other PR is about ready to go so the first commit can drop here ?

criticalAY · 2024-10-02T15:47:41Z

I will update it to use webview then /will rebase once the blocker goes in /

mikehardy · 2024-10-02T16:33:43Z

this one is free / unblocked now 🥳

mikehardy

This is looking really good. Love the "use webview for all the things" direction if we're going to be using a WebView at all

Was testing this and took a crash here - granted, unlikely perhaps in the wild but on an emulator, boom

add note (basic forward/reversed)
click paperclip
select gallery

10-12 12:58:40.367  4867  4867 D MultimediaImageFragment: MultimediaImageFragment:: Opening gallery
10-12 12:58:40.391   661   963 D CompatibilityChangeReporter: Compat change id reported: 182734110; UID 10193; state: ENABLED
10-12 12:58:40.392   509   608 D audioserver: FGS Logger Transaction failed
10-12 12:58:40.392   509   608 D audioserver: -129
10-12 12:58:40.399   661   963 I ActivityTaskManager: START u0 {act=android.intent.action.PICK dat=content://media/...} with LAUNCH_MULTIPLE from uid 10193 result code=-91
10-12 12:58:40.399  4867  4867 D AndroidRuntime: Shutting down VM
--------- beginning of crash
10-12 12:58:40.404  4867  4867 E AndroidRuntime: FATAL EXCEPTION: main
10-12 12:58:40.404  4867  4867 E AndroidRuntime: Process: com.ichi2.anki.debug, PID: 4867
10-12 12:58:40.404  4867  4867 E AndroidRuntime: android.content.ActivityNotFoundException: No Activity found to handle Intent { act=android.intent.action.PICK dat=content://media/... }
10-12 12:58:40.404  4867  4867 E AndroidRuntime: 	at android.app.Instrumentation.checkStartActivityResult(Instrumentation.java:2239)
10-12 12:58:40.404  4867  4867 E AndroidRuntime: 	at android.app.Instrumentation.execStartActivity(Instrumentation.java:1878)
10-12 12:58:40.404  4867  4867 E AndroidRuntime: 	at android.app.Activity.startActivityForResult(Activity.java:5589)
10-12 12:58:40.404  4867  4867 E AndroidRuntime: 	at androidx.activity.ComponentActivity.startActivityForResult(ComponentActivity.kt:704)
10-12 12:58:40.404  4867  4867 E AndroidRuntime: 	at androidx.core.app.ActivityCompat.startActivityForResult(ActivityCompat.java:244)
10-12 12:58:40.404  4867  4867 E AndroidRuntime: 	at androidx.activity.ComponentActivity$activityResultRegistry$1.onLaunch(ComponentActivity.kt:225)
10-12 12:58:40.404  4867  4867 E AndroidRuntime: 	at androidx.activity.result.ActivityResultRegistry$register$2.launch(ActivityResultRegistry.kt:137)
10-12 12:58:40.404  4867  4867 E AndroidRuntime: 	at androidx.fragment.app.Fragment$10.launch(Fragment.java:3637)
10-12 12:58:40.404  4867  4867 E AndroidRuntime: 	at androidx.activity.result.ActivityResultLauncher.launch(ActivityResultLauncher.kt:37)
10-12 12:58:40.404  4867  4867 E AndroidRuntime: 	at com.ichi2.anki.multimedia.MultimediaImageFragment.openGallery(MultimediaImageFragment.kt:328)
10-12 12:58:40.404  4867  4867 E AndroidRuntime: 	at com.ichi2.anki.multimedia.MultimediaImageFragment.handleSelectedImageOptions(MultimediaImageFragment.kt:288)
10-12 12:58:40.404  4867  4867 E AndroidRuntime: 	at com.ichi2.anki.multimedia.MultimediaImageFragment.handleImageUri(MultimediaImageFragment.kt:280)
10-12 12:58:40.404  4867  4867 E AndroidRuntime: 	at com.ichi2.anki.multimedia.MultimediaImageFragment.onViewCreated(MultimediaImageFragment.kt:272)
10-12 12:58:40.404  4867  4867 E AndroidRuntime: 	at androidx.fragment.app.Fragment.performViewCreated(Fragment.java:3152)

The "take photo, crop it, see it in the preview both times" case worked well though
I did not test the "unsupported image type" case or the actual SVG case (yet)

criticalAY · 2024-10-12T18:08:44Z

I did test it with svg, gif and normal pictures

david-allison · 2024-10-15T20:17:06Z

AnkiDroid/src/main/java/com/ichi2/anki/multimedia/MultimediaImageFragment.kt

+    private fun loadSvgFromUri(uri: Uri): String? {
+        return try {
+            val inputStream = context?.contentResolver?.openInputStream(uri)
+            inputStream?.bufferedReader()?.use { it.readText() }


I'd rather also see the inputStream in a use {. I don't believe it's necessary, but don't want readers of the code to have to think

This still doesn't close the inputStream

This is unnecessary, but removes a question I'd have each time I review the code

Index: AnkiDroid/src/main/java/com/ichi2/anki/multimedia/MultimediaImageFragment.kt IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== diff --git a/AnkiDroid/src/main/java/com/ichi2/anki/multimedia/MultimediaImageFragment.kt b/AnkiDroid/src/main/java/com/ichi2/anki/multimedia/MultimediaImageFragment.kt --- a/AnkiDroid/src/main/java/com/ichi2/anki/multimedia/MultimediaImageFragment.kt (revision 4bb6974116b35eea097c03bc1643d76582d8161b) +++ b/AnkiDroid/src/main/java/com/ichi2/anki/multimedia/MultimediaImageFragment.kt (date 1729058648238) @@ -579,11 +579,13 @@ */ private fun loadSvgFromUri(uri: Uri): String? { return try { - context?.contentResolver?.openInputStream(uri)?.bufferedReader()?.use { inputStream -> - inputStream.readText() + context?.contentResolver?.openInputStream(uri)?.use { inputStream -> + inputStream.bufferedReader().use { inputStream -> + inputStream.readText() + } } } catch (e: Exception) { - Timber.e(e, "Error reading SVG from URI") + Timber.w(e, "Error reading SVG from URI") null } }

Ahh I thought you meant it should be readable and the extra code, now use is applied to the inputStream, ensuring it is closed properly after the BufferedReader has finished reading the text.

@david-allison I already made changes, if you think this is not readable still then I will apply your patch

Waiting for your review

AnkiDroid/src/main/java/com/ichi2/anki/multimedia/MultimediaImageFragment.kt

criticalAY · 2024-11-08T20:19:59Z

@david-allison pending review

david-allison

Looks good! I think these are nitpicks, with the .use potentially being a blocker

AnkiDroid/src/main/java/com/ichi2/anki/multimedia/MultimediaImageFragment.kt

github-actions bot added the Strings label Sep 7, 2024

criticalAY added Blocked by dependency Currently blocked by some other dependent / related change Strings and removed Strings labels Sep 7, 2024

BrayanDSO approved these changes Sep 8, 2024

View reviewed changes

BrayanDSO added the Needs Second Approval Has one approval, one more approval to merge label Sep 8, 2024

github-actions bot added the Has Conflicts label Sep 10, 2024

criticalAY force-pushed the feat-render-svg branch from 2d8f019 to 167baac Compare September 10, 2024 16:36

github-actions bot removed the Has Conflicts label Sep 10, 2024

mikehardy requested changes Oct 2, 2024

View reviewed changes

github-actions bot added the Has Conflicts label Oct 2, 2024

mikehardy removed the Blocked by dependency Currently blocked by some other dependent / related change label Oct 2, 2024

lukstbit added the Needs Author Reply Waiting for a reply from the original author label Oct 4, 2024

criticalAY force-pushed the feat-render-svg branch from 167baac to cd4d47b Compare October 12, 2024 14:22

criticalAY removed the Has Conflicts label Oct 12, 2024

criticalAY force-pushed the feat-render-svg branch 2 times, most recently from 3380af6 to 654e231 Compare October 12, 2024 17:26

criticalAY added Needs Review and removed Needs Author Reply Waiting for a reply from the original author labels Oct 12, 2024

criticalAY requested a review from mikehardy October 12, 2024 17:27

mikehardy requested changes Oct 12, 2024

View reviewed changes

criticalAY force-pushed the feat-render-svg branch from 654e231 to 94cf173 Compare October 15, 2024 15:58

criticalAY requested a review from mikehardy October 15, 2024 15:58

david-allison reviewed Oct 15, 2024

View reviewed changes

criticalAY force-pushed the feat-render-svg branch from 94cf173 to 4bb6974 Compare October 16, 2024 05:36

criticalAY requested a review from david-allison October 16, 2024 05:37

criticalAY force-pushed the feat-render-svg branch from 4bb6974 to 16727fa Compare October 16, 2024 06:07

github-actions bot added the Has Conflicts label Oct 19, 2024

criticalAY force-pushed the feat-render-svg branch from 16727fa to a9bdf3a Compare November 8, 2024 20:19

criticalAY removed the Has Conflicts label Nov 8, 2024

criticalAY mentioned this pull request Nov 8, 2024

MultimediaActivity: Canvas: trying to draw too large(192000000bytes) bitmap #17378

Open

BrayanDSO self-requested a review November 8, 2024 22:42

BrayanDSO removed the Needs Second Approval Has one approval, one more approval to merge label Nov 8, 2024

github-actions bot added the Has Conflicts label Nov 9, 2024

criticalAY force-pushed the feat-render-svg branch from a9bdf3a to fd6dae6 Compare November 10, 2024 05:30

criticalAY removed the Has Conflicts label Nov 10, 2024

david-allison reviewed Nov 10, 2024

View reviewed changes

david-allison added the Needs Author Reply Waiting for a reply from the original author label Nov 10, 2024

feat: use webview to render images

1b4c10e

criticalAY force-pushed the feat-render-svg branch from fd6dae6 to 1b4c10e Compare November 16, 2024 15:44

criticalAY requested a review from david-allison November 16, 2024 15:45

criticalAY removed the Needs Author Reply Waiting for a reply from the original author label Nov 16, 2024

github-actions bot added the Has Conflicts label Nov 18, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feat[Multimedia]: render `svg` using webview #17035

Feat[Multimedia]: render `svg` using webview #17035

criticalAY commented Sep 7, 2024 •

edited

Loading

github-actions bot commented Sep 7, 2024

BrayanDSO left a comment

BrayanDSO commented Sep 9, 2024

criticalAY commented Sep 10, 2024

BrayanDSO commented Sep 10, 2024

mikehardy left a comment

criticalAY commented Oct 2, 2024

mikehardy commented Oct 2, 2024

mikehardy left a comment

criticalAY commented Oct 12, 2024

david-allison Oct 15, 2024

david-allison Oct 16, 2024

david-allison Oct 16, 2024

criticalAY Oct 16, 2024

criticalAY Oct 16, 2024

criticalAY Oct 18, 2024

criticalAY commented Nov 8, 2024

david-allison left a comment

Feat[Multimedia]: render svg using webview #17035

Are you sure you want to change the base?

Feat[Multimedia]: render svg using webview #17035

Conversation

criticalAY commented Sep 7, 2024 • edited Loading

Purpose / Description

Fixes

Approach

How Has This Been Tested?

Checklist

github-actions bot commented Sep 7, 2024

BrayanDSO left a comment

Choose a reason for hiding this comment

BrayanDSO commented Sep 9, 2024

criticalAY commented Sep 10, 2024

BrayanDSO commented Sep 10, 2024

mikehardy left a comment

Choose a reason for hiding this comment

criticalAY commented Oct 2, 2024

mikehardy commented Oct 2, 2024

mikehardy left a comment

Choose a reason for hiding this comment

criticalAY commented Oct 12, 2024

david-allison Oct 15, 2024

Choose a reason for hiding this comment

david-allison Oct 16, 2024

Choose a reason for hiding this comment

david-allison Oct 16, 2024

Choose a reason for hiding this comment

criticalAY Oct 16, 2024

Choose a reason for hiding this comment

criticalAY Oct 16, 2024

Choose a reason for hiding this comment

criticalAY Oct 18, 2024

Choose a reason for hiding this comment

criticalAY commented Nov 8, 2024

david-allison left a comment

Choose a reason for hiding this comment

Feat[Multimedia]: render `svg` using webview #17035

Feat[Multimedia]: render `svg` using webview #17035

criticalAY commented Sep 7, 2024 •

edited

Loading