[Bug] Incompatible behaviour between Anki-desktop and Anki-Android regarding percent-escape of URL in <object>

chenlijun99 commented 1 year ago

Reproduction Steps

Create a note with <object data="hello100%.pdf"></object> as content. Anki-Android doesn't escape, while Anki-desktop escapes. It is not the case for <img src>, where both Anki-desktop and Anki-Android escape the URL.

I believe the problem lies in this function https://github.com/ankidroid/Anki-Android/blob/7c9890327e80ac86b3b63b7d30ff8e208da8eef7/AnkiDroid/src/main/java/com/ichi2/libanki/Media.kt#L952-L975. Probably also fObjectRegExpQ and fObjectRegExpU should be included in the listOf, but I'm not enough familiar with the codebase to tell.

Expected Result

Same behavior

Actual Result

Anki-Android doesn't escape, while Anki-desktop escapes.

Debug info

AnkiDroid Version = 2.16alpha89

Android Version = 12

Manufacturer = samsung

Model = SM-A528B

Hardware = qcom

Webview User Agent = Mozilla/5.0 (Linux; Android 12; SM-A528B Build/SP1A.210812.016; wv) AppleWebKit/537.36 (KHTML, like Gecko) Version/4.0 Chrome/107.0.5304.105 Mobile Safari/537.36

ACRA UUID = 090a732e-ed41-4628-b63e-9ede75d125ae

New schema = false

Scheduler = std2

Crash Reports Enabled = true

DatabaseV2 Enabled = true

Research

Enter an [x] character to confirm the points below:

[x] I have read the support page and am reporting a bug or enhancement request specific to AnkiDroid
[x] I have checked the manual and the FAQ and could not find a solution to my issue
[x] I have searched for similar existing issues here and on the user forum
[x] (Optional) I have confirmed the issue is not resolved in the latest alpha release (instructions)

welcome[bot] commented 1 year ago

Hello! 👋 Thanks for logging this issue. Please remember we are all volunteers here, so some patience may be required before we can get to the issue. Also remember that the fastest way to get resolution on an issue is to propose a change directly, https://github.com/ankidroid/Anki-Android/wiki/Contributing

mikehardy commented 1 year ago

Hi there! I don't see the version you are on, can you include the info requested here:

Refer to the support page if you are unsure where to get the "debug info".

(and if you are not on the latest alphas, could you give them a try - especially with "new schema" and "v3 scheduler" support enabled in advanced settings to see if they have an effect https://github.com/ankidroid/Anki-Android/releases )

chenlijun99 commented 1 year ago

Hi. I've added the initial debug infos.

I've also tested the latest alpha and enabled the suggested settings. But the problem persists.

Updated debug info

AnkiDroid Version = 2.16alpha90

Android Version = 12

Manufacturer = samsung

Model = SM-A528B

Hardware = qcom

Webview User Agent = Mozilla/5.0 (Linux; Android 12; SM-A528B Build/SP1A.210812.016; wv) AppleWebKit/537.36 (KHTML, like Gecko) Version/4.0 Chrome/107.0.5304.105 Mobile Safari/537.36

ACRA UUID = 090a732e-ed41-4628-b63e-9ede75d125ae

New schema = true

Scheduler = std3

Crash Reports Enabled = true

DatabaseV2 Enabled = true

mikehardy commented 1 year ago

I'm sorry it's still happening! Thanks for updating the info and for trying the alphas of 2.16, we really appreciate it. I've tagged this up for investigation so we can hopefully get to the bottom of it

criticalAY commented 1 year ago

AnkiDesktop escapes both object and image url but I don't think Ankidroid escapes them both though there is a snackback in case we insert the tag by ourself and there is no image as such existing but nothing pops up for the object

chenlijun99 commented 1 year ago

My case is that I have JavaScript that dynamically loads and renders resources referenced in <object>. But for now I just have special logic that encodes the URL read from the data property of a HTMLObjectElement, if the JavaScript is executing on Anki-Android. See https://github.com/chenlijun99/autoanki/blob/379af2927eee53df0bb8d007cda0813308c2c111/packages/autoanki-anki-bridge/src/index.ts#L8-L34

But this sort of incompatibility should be eventually clarified. Maybe involving some developer of Anki-Desktop in the conversation could be also useful to understand what exactly should be done, as maybe the problem is not on the Anki-Android side. For example, if I've understood correctly, the way Anki-Desktop and partially also Anki-Android just unconditionally encodes the media URL means that there is a limitation: we can't have query parameters and URL fragments in the media URL. E.g. if I manually wrote <object data="myfile.pdf#page=1"></object> in the HTML editor, it will be transformed into <object data="myfile.pdf%23page=1"></object>.

justdvnsh commented 1 year ago

@mikehardy Why are we not including fObjectRegExpQ and fObjectRegExpU in the listOf in Media.kt escapeImages() function ? Is there any specific reason to it ? Currently, I see we already have created the regex for escaping the object tags as well,

private val fObjectRegExpQ = Pattern.compile("(?i)(<object\\b[^>]* data=([\"'])([^>]+?)(\\2)[^>]*>)") private val fObjectRegExpU = Pattern.compile("(?i)(<object\\b[^>]* data=(?!['\"])([^ >]+)[^>]*?>)")

but apparently we aren't using it.

If we include those regex in the listOf in escapeImages() function, then the app is behaving as expected like in desktop.

The old response (without using the object url escaping) question: '<style>.card { font-family: arial; font-size: 20px; text-align: center; color: black; background-color: white; } </style><object data="hello100%.pdf"></object>'

The new response (after using the object url escaping regex along) question: '<style>.card { font-family: arial; font-size: 20px; text-align: center; color: black; background-color: white; } </style><object data="hello100%25.pdf"></object>'

mikehardy commented 1 year ago

@justdvnsh I'm not sure why, I can only counsel as a general thing to use git blame, we went to great pains to make sure that git blame worked even through the java -> kotlin transformation so you should be able to see commits where that code was worked on. If there is no reasonable explanation or if it's just an obvious oversight (looks like it was an obvious oversight?) then fixing it as you describe may be the correct PR

github-actions[bot] commented 1 year ago

Hello 👋, this issue has been opened for more than 2 months with no activity on it. If the issue is still here, please keep in mind that we need community support and help to fix it! Just comment something like still searching for solutions and if you found one, please open a pull request! You have 7 days until this gets closed automatically

ankidroid / Anki-Android