Skip to content

fix: detect .svg uris with a query, fragment or upper case extension - #1091

Open
afonsograca wants to merge 1 commit into
gmsgowtham:mainfrom
afonsograca:fix/svg-uri-detection
Open

fix: detect .svg uris with a query, fragment or upper case extension#1091
afonsograca wants to merge 1 commit into
gmsgowtham:mainfrom
afonsograca:fix/svg-uri-detection

Conversation

@afonsograca

Copy link
Copy Markdown
Contributor

As a user, I should see an SVG image when its address has a query string or a fragment. Many SVG assets come from a CDN. These addresses often hold a signature or a cache-busting parameter, for example logo.svg?v=1. Today these images are blank.

In this PR we correct the test that finds an SVG address:

  • Renderer.image() tested uri.endsWith(".svg"). An address with a query string or a fragment failed this test. The address then went to MDImage. MDImage cannot draw an SVG file, and the image was blank.
  • The new test removes the query string and the fragment first. It then compares the extension, and it ignores the letter case. The addresses logo.svg?v=1, LOGO.SVG and logo.svg#top now go to MDSvg.
  • We add two tests next to the alt text tests. One test uses https://example.com/logo.svg?v=1. The other test uses https://example.com/LOGO.SVG#top. Both tests use the existing fetch mock, and both tests check that MDSvg renders.
  • No snapshot changes. The SVG examples in Markdown.spec.tsx use a plain .svg address.

The predicate is /\.svg$/i.test(uri.split(/[?#]/, 1)[0] ?? ""). The repository sets noUncheckedIndexedAccess, so the index gives string | undefined. The ?? "" gives RegExp.test() a string.

Renderer.image() is the only place that finds an SVG address, so one change is sufficient.

The new tests use the fetch mock that #1089 added.

Renderer.image() tested uri.endsWith(".svg"). Addresses such as
logo.svg?v=1 or LOGO.SVG went to MDImage, which cannot draw an SVG,
and the image was blank. Remove the query string and the fragment
before the test, and ignore the letter case.
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.

1 participant