-
-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
Support dimension bindings in cross-origin mode #2989
Conversation
src/runtime/internal/dom.ts
Outdated
if (crossorigin === undefined) { | ||
try { | ||
if (typeof window === 'undefined' || !window.parent || window.parent.document) { | ||
crossorigin = false; |
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.
Is it possible that none of these conditions will be true, and yet an exception is not thrown? If yes, crossorigin
wouldn't get set in those cases. If not, could this be made a little clearer?
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.
Either the last one throws, or one of them is true, but I'm not sure how to make it clearer. I suppose crossorigin = false
could come before the tests, would that help? Perhaps adding a void
to signify it's done on purpose instead of a forgotten statement?
if (crossorigin === undefined) {
crossorigin = false;
try {
if (typeof window !== 'undefined' && window.parent) {
void window.parent.document;
}
} catch (error) {
crossorigin = true;
}
}
return crossorigin;
Would this mean that the workaround in #2279 would no longer be necessary? |
Actually, there are probably still some other things that people might want to do that require more relaxed sandboxing. Having a UI toggle for this is probably still a good TODO. |
I:
and the size binding did not seem to be working. Did I do something wrong to test this? |
Ah, interesting. I didn't notice it wasn't working for |
@mrkishi Do you think you might have time to take another look at this? |
ed7a3f3
to
3ac8b7a
Compare
Took me a while (...just a little bit), but finally moved it back to mounting phase - now working on static elements. |
|
||
if (/Trident/.test(navigator.userAgent)) { |
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 don't know what browser difference this UA sniffing was originally meant to work around, but can you confirm that it's no longer necessary?
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'm honestly not sure. We're using iframe
now and I tested it on Chrome/Firefox/Edge/Internet Explorer, but this might still make a difference on older IEs?
Hello. I was on version But I can't relate to the cross-origin thing... I don't use iframes, I'm simply using my dev environment, from a URL such as http://myapp.localhost:5000/ Why would it now work? Didn't you do more than supporting something related to cross origin? |
bind:clientWidth
and friends currently don't work when you're under cross-origin restrictions. This manifests itself in the REPL or any time a component with those bindings is used in a cross-originiframe
.This PR just relies on message passing in these cases. The only browser that doesn't support this is IE11, but that's because it doesn't support
srcdoc
orsrc="data:"
, so I don't know what our options are. It didn't work before either, so it's not a regression.As a small bonus, I made the helper follow the regular listener/bindings format, so it now reuses the
dispose
array with them.Closes #2147 and sveltejs/svelte-repl#12.