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

chore(build): remove the redirection of legacy images that start with /@api/deki/ #11228

Merged
merged 1 commit into from
Jun 5, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 2 additions & 3 deletions build/check-images.ts
Original file line number Diff line number Diff line change
Expand Up @@ -89,11 +89,10 @@ export function checkImageReferences(

// These two lines is to simulate what a browser would do.
const baseURL = `http://yari.placeholder${url}/`;
// Make a special exception for the legacy images that start with `/@api/deki...`
// Make a special exception for the legacy images that start with `/files/...`
// If you just pretend their existing URL is static external domain, it
// will be recognized as an external image (which is fixable).
// Also any `<img src="/files/1234/foo.png">` should match.
const absoluteURL = /^\/(@api\/deki\/|files\/\d+)/.test(src)
const absoluteURL = /^\/files\/\d+/.test(src)
? new URL(`https://mdn.mozillademos.org${src}`)
: new URL(src, baseURL);
Comment on lines +95 to 97
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think we need to support the other case either, as mdn.mozillademos.org has been decommissioned. Or do we still have links to /files?

Suggested change
const absoluteURL = /^\/files\/\d+/.test(src)
? new URL(`https://mdn.mozillademos.org${src}`)
: new URL(src, baseURL);
const absoluteURL = new URL(src, baseURL);

Copy link
Member Author

Choose a reason for hiding this comment

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

Copy link
Contributor

Choose a reason for hiding this comment

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

@fiji-flo Will rari support this? (I guess not, which means we should migrate those in translated-content.)


Expand Down
4 changes: 0 additions & 4 deletions build/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -110,10 +110,6 @@ export async function downloadAndResizeImage(
// has to be escaped when working on the command line.
.replace(/[()]/g, "")
.replace(/\s+/g, "_")
// From legacy we have a lot of images that are named like
// `/@api/deki/files/247/=HTMLBlinkElement.gif` for example.
// Take this opportunity to clean that odd looking leading `=`.
.replace(/^=/, "")
.toLowerCase()
);
// Before writing to disk, run it through the same imagemin
Expand Down
10 changes: 0 additions & 10 deletions testing/integration/headless/test_cdn.py
Original file line number Diff line number Diff line change
Expand Up @@ -178,16 +178,6 @@ def test_cached(base_url, is_behind_cdn, slug, status, expected_location):
assert_cached(base_url + slug, status, expected_location, is_behind_cdn)


@pytest.mark.parametrize(
"slug,status",
[("/files/2767/hut.jpg", 301), ("/@api/deki/files/3613/=hut.jpg", 301)],
)
def test_cached_attachments(base_url, attachment_url, is_behind_cdn, slug, status):
"""Ensure that these file-attachment requests are cached."""
expected_location = attachment_url + slug
assert_cached(base_url + slug, status, expected_location, is_behind_cdn)


@pytest.mark.parametrize(
"zone,status,expected_location",
[
Expand Down