-
Notifications
You must be signed in to change notification settings - Fork 3.5k
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
Added referrer policy support to GUI Image to control xhr request header #12664
Conversation
Please make sure to label your PR with "bug", "new feature" or "breaking change" label(s). |
There is no need to update the "what's new.md" file. A changelog will be generated using the PR and its tags. |
Please make sure to label your PR with "bug", "new feature" or "breaking change" label(s). |
Snapshot stored with reference name: Test environment: To test a playground add it to the URL, for example: https://babylonsnapshots.z22.web.core.windows.net/refs/pull/12664/merge/index.html#WGZLGJ#4600 Links to test babylon tools with this snapshot: https://playground.babylonjs.com/?snapshot=refs/pull/12664/merge To test the snapshot in the playground with a playground ID add it after the snapshot query string: https://playground.babylonjs.com/?snapshot=refs/pull/12664/merge#BCU1XR#0 |
Please make sure to label your PR with "bug", "new feature" or "breaking change" label(s). |
Snapshot stored with reference name: Test environment: To test a playground add it to the URL, for example: https://babylonsnapshots.z22.web.core.windows.net/refs/pull/12664/merge/index.html#WGZLGJ#4600 Links to test babylon tools with this snapshot: https://playground.babylonjs.com/?snapshot=refs/pull/12664/merge To test the snapshot in the playground with a playground ID add it after the snapshot query string: https://playground.babylonjs.com/?snapshot=refs/pull/12664/merge#BCU1XR#0 |
Adding @RaananW as a reviewer who is way more versed that I am on this part :-) I love the overall idea but wonder about persistence as well as should we do it for the 3d textures and much more ? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it's more of a question of whether or not this is the right place for this.
We already have Tools.SetCorsBehavior
, which changes the crossOrigin
of the specific request/image. We also have the WebRequest modifiers to change any request made ourside of a simple image load. This feels like a 3rd way to solve a similar issue.
I like the idea of adding it, but we need to find a global way to be able to set this. Only in the GUI is probably not suffecient.
Hello ! I'm not sure for the serialization, so I removed it, I will put it back if you think it's relevant. |
Please make sure to label your PR with "bug", "new feature" or "breaking change" label(s). |
Snapshot stored with reference name: Test environment: To test a playground add it to the URL, for example: https://babylonsnapshots.z22.web.core.windows.net/refs/pull/12664/merge/index.html#WGZLGJ#4600 Links to test babylon tools with this snapshot: https://playground.babylonjs.com/?snapshot=refs/pull/12664/merge To test the snapshot in the playground with a playground ID add it after the snapshot query string: https://playground.babylonjs.com/?snapshot=refs/pull/12664/merge#BCU1XR#0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
@sebavan, any objection merging this?
This feature will allow user's to control the header in the xhr request to load the image,
Referrer Policy
.More info here :
The Referrer-Policy HTTP header
Default value is :
strict-origin-when-cross-origin