-
Notifications
You must be signed in to change notification settings - Fork 27
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
Add cross-origin window and location wrappers #291
Conversation
Closes dart-lang/sdk#54443 Closes dart-lang#247 Closes dart-lang/sdk#54938 Since cross-origin objects have limitations around access, wrappers are introduced to do the only safe operations. Extension methods are added to get instances of these wrappers.
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!
@@ -36,4 +39,65 @@ void main() { | |||
// Ensure accessing any arbitrary item in the list does not throw. | |||
expect(() => dartList[0], returnsNormally); | |||
}); | |||
|
|||
test('cross-origin windows and locations can be accessed safely', () { |
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.
(optional) I wonder if it would be worth to also add test coverage of the unwrapped behavior? That is, something that verifies that cross-origin windows/locations fail if you tried to access them directly?
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.
Agreed it'd be useful to verify.
In the process of adding this, however, I noticed package:test
doesn't quite work as expected. Running a known same-origin policy violation e.g. accessing navigator
on an opened window from a different origin only fails if you --pause-after-load
and single-step through the test. Without that, opened windows' origins appears to be the same as the opening window's origin (localhost
). I know there's some setup to load these tests in an iframe to run them, but I can't tell how it results in the above behavior. The current test passes regardless of if you single-step, so for now, I put a TODO.
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 both the test runner and package:test
have this issue. After a good amount of debugging, I can't tell how to enable same-origin policy. Filed dart-lang/test#2282 for now and single-stepped to verify that there are no cross-origin issues with the current test.
Also need to rebase on latest...although it looks like we're going for a 1.1 release soon. |
Closes dart-lang/sdk#54443
Closes #247
Closes dart-lang/sdk#54938
Since cross-origin objects have limitations around access, wrappers are introduced to do the only safe operations. Extension methods are added to get instances of these wrappers.
I'll work on getting a test in the SDK doing the interop operations with
JSAny?
so that ability we're relying on here won't regress and we can get better coverage.