Skip to content

Fetch the image as a blob before loading to properly handle CORS#5430

Open
jpggvilaca wants to merge 4 commits intodevelopfrom
jvilaca/fix-media-loading-issue
Open

Fetch the image as a blob before loading to properly handle CORS#5430
jpggvilaca wants to merge 4 commits intodevelopfrom
jvilaca/fix-media-loading-issue

Conversation

@jpggvilaca
Copy link
Contributor

@jpggvilaca jpggvilaca commented Feb 5, 2026

Summary

Since the frontend and backend run on different ports (frontend likely on a dev server port, backend on 7860), they're different origins. The `crossOrigin = 'anonymous' is needed to read image data for canvas operations, but it requires proper CORS headers from the backend.

Now we fetch the image as a blob first, which will go through the CORS middleware correctly.

Trial & error plus copilot saved the day.

Please do test manually by opening media items multiples times and refreshing the page

How to test

Checklist

  • The PR title and description are clear and descriptive
  • I have manually tested the changes
  • All changes are covered by automated tests
  • All related issues are linked to this PR (if applicable)
  • Documentation has been updated (if applicable)

@jpggvilaca jpggvilaca requested a review from a team as a code owner February 5, 2026 07:40
Copilot AI review requested due to automatic review settings February 5, 2026 07:40
@jpggvilaca jpggvilaca added the Geti Tune UI Issues related to Geti Tune UI label Feb 5, 2026
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR updates the image loading mechanism to properly handle CORS by fetching images as blobs before loading them into Image elements, resolving cross-origin issues between the frontend dev server and backend running on different ports.

Changes:

  • Modified loadImage function to fetch images as blobs via the fetch API with CORS credentials
  • Added proper cleanup of object URLs after image loading completes
  • Preserved existing test environment behavior with early return path

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@github-actions
Copy link

github-actions bot commented Feb 5, 2026

📊 Test coverage report

Metric Coverage
Lines 36.7%
Functions 76.4%
Branches 88.2%
Statements 36.7%

@github-actions
Copy link

github-actions bot commented Feb 5, 2026

Docker Image Sizes

CPU

Image Size
geti-tune-cpu:pr-5430 965.70M
geti-tune-cpu:sha-13e3b31 965.70M

GPU

Image Size
geti-tune-gpu:pr-5430 5.51G
geti-tune-gpu:sha-13e3b31 5.51G

XPU

Image Size
geti-tune-xpu:pr-5430 3.30G
geti-tune-xpu:sha-13e3b31 3.30G

@dwesolow
Copy link
Contributor

dwesolow commented Feb 5, 2026

it requires proper CORS headers from the backend

Why cannot we set proper headers then?

export const loadImage = (link: string): Promise<HTMLImageElement> =>
new Promise<HTMLImageElement>((resolve, reject) => {
export const loadImage = async (link: string): Promise<HTMLImageElement> => {
if (process.env.NODE_ENV === 'test') {
Copy link
Contributor

Choose a reason for hiding this comment

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

In general, I don’t like this approach, we shouldn’t have to add code just to make the test pass.

Copy link
Contributor

Choose a reason for hiding this comment

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

I agree with that. Can we mock this function on the test side?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This code is like 3 years old. Had to do with the speed we load images when loading tests. But i agree. Deleted.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Geti Tune UI Issues related to Geti Tune UI

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants