Use non-canonical URL for local non-model assets #6082
Conversation
|
Thanks for working on it.
Please use draft PR if a PR is not ready. |
Yes, I have been trying to but Github is throwing 500 Errors on their servers with the "convert to draft" route. It is a known outage. I will just close for now and reopen when GH is stable again and can convert to Draft. |
| @@ -615,10 +615,9 @@ export async function resolveMediaInfo(urlString) { | |||
|
|
|||
| // We want to resolve and proxy some hubs urls, like rooms and scene links, | |||
| // but want to avoid proxying assets in order for this to work in dev environments | |||
| const isLocalModelAsset = | |||
| isNonCorsProxyDomain(url.hostname) && (guessContentType(url.href) || "").startsWith("model/gltf"); | |||
There was a problem hiding this comment.
@johnshaughnessy do you have any memory of why models were the only local assets that don't use a canonical URL? With the newLoader path - uploaded images, movies, and sounds are broken links when working in a local dev env.
I know this line also appears in "components/media-loader.js" which dates back to this PR.
If I make the change shown here then all media tested (glb, mp3, mp4, jpg) work with newLoader in local dev and on my personal instance. That said, I am unsure of the original intent in having non-model assets use canonical urls and don't want to introduce a regression.
There was a problem hiding this comment.
Asking you specifically because you were the last to work on this section. It was 9 months ago though so no worries if you don't remember.
There was a problem hiding this comment.
Hmm, unfortunately no. I don't know why we made an exception only for models. But, given that we're OK with the exception for models, I can't think of why we wouldn't be OK to lift the restriction for other media types like videos and images.
At the risk of some regression, I'll go ahead with this PR (despite not understanding what lies on the other side of the fence).
stalgiag
changed the title
johnshaughnessy
force-pushed
the
local-image-fix
branch
from
0a28eae to
5bcff45
Compare
Links for local files are broken when working locally behind
newLoaderflag.This change causes the canconical URL to be retrieved for non-model local assets. This is tested locally and on my personal dev instance with *.jpg, *.glb, *.mp4, and *.mp3. It is possible that there is something that I am missing and that this is a regression so this should not be merged until I can confirm what the original intent was for limiting this behavior to model assets.
resolves #6081