Skip to content
This repository has been archived by the owner on Nov 29, 2023. It is now read-only.

Fix: ensure dynamic manifest URLs are mapped #360

Merged

Conversation

eyelidlessness
Copy link
Contributor

Closes enketo/enketo#1036

I have verified this PR works with

  • Online form submission
  • Offline form submission
  • Saving offline drafts
  • Submitting offline drafts
  • Editing submissions
  • Form preview
  • [ ] None of the above

What else has been done to verify that this works as intended?

  • Loaded a form with binary defaults in a public access link (logged out, in Incognito mode) to verify that the media loads with the form's dynamic public access token.
  • Validated that transformer escapes the same special HTML/XML characters with the same entities
  • Validated that spaces and other special URL characters continue to be escaped and mapped as expected

Why is this the best possible solution? Were any other approaches considered?

This is a slightly safer (escaping reserved HTML/XML entities, still handles spaces and other URL characters that need to be escaped) version of the previous solution, before #291 and was determined to be the lowest risk short-term solution. Other solutions considered:

  • Dynamically map media in the frontend DOM on load. This turned out to be unexpectedly complex.
  • Transform for each distinct manifest. This would likely be a performance regression.

How does this change affect users? Describe intentional changes to behavior and behavior that could have accidentally been affected by code changes. In other words, what are the regression risks?

  • It's possible, though probably unlikely, that the string replacement approach will find false positives.

Do we need any specific form for testing your changes? If so, please attach one.

Any form with binary defaults (currently only for annotation fields, see enketo/enketo#1033).

} );


it( 'escapes media URLs in markdown linkes', async () => {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ye old linkes

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Media URIs are cached on first form load so dynamic manifest URLs are ignored
2 participants