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

Set crossorigin attribute on the default image in pixel viewer #1254

Merged
merged 4 commits into from Apr 22, 2020

Conversation

courtneycb
Copy link
Contributor

@courtneycb courtneycb commented Apr 22, 2020

Inconsistencies have been occurring on the pixel viewer interactive regarding CORS requests. After some investigating we noticed:

  • Upon disabling cache through dev tools we no longer got a CORS error.
  • Two requests for images were being made; one of which was being made through jquery.
  • In the response headers there is an attribute sec-fetch-mode that has the value no-cors.
  • Only the default image was triggering the CORS error.

We suspect that the jquery request is being cached first (which doesn't have CORS headers), and then the second request is made (with CORS headers) but during this request Chrome fetches from the cache (but the CORS headers aren't cached due to the first request) which results in a CORS error on the second fetch.

With this change we see only one request being made and it appears to have the sec-fetch-mode attribute set to cors locally.

@courtneycb courtneycb requested a review from eAlasdair Apr 22, 2020
@courtneycb courtneycb self-assigned this Apr 22, 2020
@courtneycb
Copy link
Contributor Author

@courtneycb courtneycb commented Apr 22, 2020

Closing in favour of #1255

@courtneycb courtneycb closed this Apr 22, 2020
@courtneycb courtneycb reopened this Apr 22, 2020
@eAlasdair
Copy link
Collaborator

@eAlasdair eAlasdair commented Apr 22, 2020

So we want these changes (if successful) to be merged into the master branch, with none of the other changes that have been made to develop since they last split. So:

1 We merge the change into develop (This PR)
Then IF it works:
2 We merge #1255 (with only these changes but from the master branch) into master
3 We update #1255 to be inline with develop
4 We merge #1255 to develop

Edit We have been ordered to do it as a normal release

@courtneycb courtneycb merged commit 57c6b19 into develop Apr 22, 2020
5 checks passed
@courtneycb courtneycb deleted the investigate-pixel-viewer branch Apr 22, 2020
@courtneycb courtneycb mentioned this pull request Apr 22, 2020
@uccser-bot uccser-bot mentioned this pull request May 4, 2020
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.

None yet

2 participants