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

Make crossOriginObject.then undefined for promises #3242

Merged
merged 2 commits into from
Feb 7, 2018

Conversation

annevk
Copy link
Member

@annevk annevk commented Nov 20, 2017

This allows promises to work better with cross-origin WindowProxy and Location objects.

Tests: (please write some).

Fixes: whatwg/dom#536.


/browsers.html ( diff )

This allows promises to work better with cross-origin WindowProxy and Location objects.

Tests: (please write some).

Fixes: whatwg/dom#536.
source Outdated
@@ -77875,6 +77868,14 @@ console.assert(iframeWindow.frameElement === null);
</ol>
</li>

<li><p>If <var>P</var> is "<code data-x="">then</code>", <span>@@toStringTag</span>,
<span>@@hasInstance</span>, or <span>@@isConcatSpreadable</span>, then return
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I noticed a weird thing that above we use SameValue when comparing P but here we make do with "is". Does ECMAScript have some kind of pattern for this that might be better? (Not a new issue though, so not a blocker in my opinion.)

MXEBot pushed a commit to mirror/chromium that referenced this pull request Nov 23, 2017
According to the discussion (and request) at
  whatwg/dom#536
this patch adds 'then' as a cross origin accessible property
returning 'undefined'.

HTML 7.2.3.3 CrossOriginGetOwnPropertyHelper ( O, P )
https://html.spec.whatwg.org/multipage/browsers.html#crossorigingetownpropertyhelper-(-o,-p-)
is not yet updated, but it should be quite soon.  The proposed
change is at
  whatwg/html#3242

Bug: 
Change-Id: Icb72ba01ccf8eabaad607b61805411d1b4845f86
Reviewed-on: https://chromium-review.googlesource.com/779319
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Commit-Queue: Yuki Shiino <yukishiino@chromium.org>
Cr-Commit-Position: refs/heads/master@{#518158}
Copy link
Member

@domenic domenic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Change LGTM, just waiting for tests

@domenic domenic added normative change needs tests Moving the issue forward requires someone to write tests labels Dec 1, 2017
@annevk
Copy link
Member Author

annevk commented Feb 4, 2018

@annevk annevk added the interop Implementations are not interoperable with each other label Feb 4, 2018
@bzbarsky
Copy link
Contributor

bzbarsky commented Feb 7, 2018

@domenic I will add some tests in https://bugzilla.mozilla.org/show_bug.cgi?id=1435596 so you should go ahead and merge this. If you wanted to, you could land the test changes from that patch in wpt, I guess, instead of waiting for us to merge them back.

I tried running those tests in Chrome, but Chrome seems to fail most of the tests in wpt's html/browsers/origin/cross-origin-objects/cross-origin-objects.html.... It does pass the specific tests I added for Promise.resolve() with cross-origin objects, but not any of the tests about the actual internal methods, because it seems to be doing nothing even resembling what the spec says. :(

@domenic domenic removed the needs tests Moving the issue forward requires someone to write tests label Feb 7, 2018
@domenic domenic merged commit 6031e3a into master Feb 7, 2018
@domenic domenic deleted the annevk/cross-origin-objects-then branch February 7, 2018 19:39
kisg pushed a commit to paul99/webkit-mips that referenced this pull request Oct 1, 2018
https://bugs.webkit.org/show_bug.cgi?id=190094

Reviewed by Darin Adler.

LayoutTests/imported/w3c:

Rebaseline WPT test now that more checks are passing.

* web-platform-tests/html/browsers/origin/cross-origin-objects/cross-origin-objects-expected.txt:

Source/WebCore:

Make crossOriginObject.then undefined for promises. This allows promises to work better with cross-origin WindowProxy
and Location objects.

Specification:
- whatwg/html#3242
- whatwg/dom#536

This aligns our behavior with Blink and Gecko.

No new tests, rebaselined existing test.

* bindings/js/JSDOMWindowCustom.cpp:
(WebCore::jsDOMWindowGetOwnPropertySlotRestrictedAccess):
(WebCore::addCrossOriginWindowOwnPropertyNames):
* bindings/js/JSLocationCustom.cpp:
(WebCore::getOwnPropertySlotCommon):
(WebCore::addCrossOriginLocationOwnPropertyNames):

LayoutTests:

Update existing tests to reflect behavior change.

* http/tests/navigation/process-swap-window-open-expected.txt:
* http/tests/navigation/process-swap-window-open.html:
* http/wpt/cross-origin-window-policy/resources/utils.js:
(testCrossOriginOption):


git-svn-id: http://svn.webkit.org/repository/webkit/trunk@236661 268f45cc-cd09-0410-ab3c-d52691b4dbfc
ryanhaddad pushed a commit to WebKit/WebKit that referenced this pull request Dec 22, 2020
https://bugs.webkit.org/show_bug.cgi?id=190094

Reviewed by Darin Adler.

LayoutTests/imported/w3c:

Rebaseline WPT test now that more checks are passing.

* web-platform-tests/html/browsers/origin/cross-origin-objects/cross-origin-objects-expected.txt:

Source/WebCore:

Make crossOriginObject.then undefined for promises. This allows promises to work better with cross-origin WindowProxy
and Location objects.

Specification:
- whatwg/html#3242
- whatwg/dom#536

This aligns our behavior with Blink and Gecko.

No new tests, rebaselined existing test.

* bindings/js/JSDOMWindowCustom.cpp:
(WebCore::jsDOMWindowGetOwnPropertySlotRestrictedAccess):
(WebCore::addCrossOriginWindowOwnPropertyNames):
* bindings/js/JSLocationCustom.cpp:
(WebCore::getOwnPropertySlotCommon):
(WebCore::addCrossOriginLocationOwnPropertyNames):

LayoutTests:

Update existing tests to reflect behavior change.

* http/tests/navigation/process-swap-window-open-expected.txt:
* http/tests/navigation/process-swap-window-open.html:
* http/wpt/cross-origin-window-policy/resources/utils.js:
(testCrossOriginOption):


Canonical link: https://commits.webkit.org/205090@main
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@236661 268f45cc-cd09-0410-ab3c-d52691b4dbfc
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
interop Implementations are not interoperable with each other normative change
Development

Successfully merging this pull request may close these issues.

3 participants