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

Enumerate fewer cross-origin properties #8045

Merged
merged 2 commits into from Nov 4, 2017

Conversation

Projects
None yet
5 participants
@annevk
Member

annevk commented Nov 2, 2017

See whatwg/html#3186 for context.

@wpt-pr-bot wpt-pr-bot added the html label Nov 2, 2017

@wpt-pr-bot wpt-pr-bot requested review from jdm, jgraham and zqzhang Nov 2, 2017

@annevk

This comment has been minimized.

Show comment
Hide comment
@annevk

annevk Nov 2, 2017

Member

So the one thing I don't get is that in https://bug1412741.bmoattachments.org/attachment.cgi?id=8924054 @bzbarsky also marked the [[OwnPropertyKeys]] test as having changed results. I don't see how that follows from the standard.

Member

annevk commented Nov 2, 2017

So the one thing I don't get is that in https://bug1412741.bmoattachments.org/attachment.cgi?id=8924054 @bzbarsky also marked the [[OwnPropertyKeys]] test as having changed results. I don't see how that follows from the standard.

@w3c-bots

This comment has been minimized.

Show comment
Hide comment
@w3c-bots

w3c-bots Nov 2, 2017

Build PASSED

Started: 2017-11-03 10:59:45
Finished: 2017-11-03 11:03:23

View more information about this build on:

w3c-bots commented Nov 2, 2017

Build PASSED

Started: 2017-11-03 10:59:45
Finished: 2017-11-03 11:03:23

View more information about this build on:

@bzbarsky

This comment has been minimized.

Show comment
Hide comment
@bzbarsky

bzbarsky Nov 2, 2017

Contributor

also marked the [[OwnPropertyKeys]] test as having changed results

The test in question is http://searchfox.org/mozilla-central/rev/423b2522c48e1d654e30ffc337164d677f934ec3/testing/web-platform/tests/html/browsers/origin/cross-origin-objects/cross-origin-objects.html#283-296 (or https://github.com/w3c/web-platform-tests/blob/26cc7b48a87800a0e28ea765e3630b1c929cf1c9/html/browsers/origin/cross-origin-objects/cross-origin-objects.html#L283-L296 if you prefer). It tests the two obvious spec entrypoints that exercise [[OwnPropertyKeys]]: Object.getOwnPropertyNames and Object.keys. The former is not affected by enumerability, but the latter is.

I didn't want to just change the test before the spec changes, so I just marked it as failing for now, but of course we'll want to update the test...

Contributor

bzbarsky commented Nov 2, 2017

also marked the [[OwnPropertyKeys]] test as having changed results

The test in question is http://searchfox.org/mozilla-central/rev/423b2522c48e1d654e30ffc337164d677f934ec3/testing/web-platform/tests/html/browsers/origin/cross-origin-objects/cross-origin-objects.html#283-296 (or https://github.com/w3c/web-platform-tests/blob/26cc7b48a87800a0e28ea765e3630b1c929cf1c9/html/browsers/origin/cross-origin-objects/cross-origin-objects.html#L283-L296 if you prefer). It tests the two obvious spec entrypoints that exercise [[OwnPropertyKeys]]: Object.getOwnPropertyNames and Object.keys. The former is not affected by enumerability, but the latter is.

I didn't want to just change the test before the spec changes, so I just marked it as failing for now, but of course we'll want to update the test...

@annevk

This comment has been minimized.

Show comment
Hide comment
@annevk

annevk Nov 3, 2017

Member

Thanks, I'll fix that one too. I suspect it would be better for @cdumez et al if we fixed it here straight away given that there's still some (significant) delay if we land web-platform-tests changes from the Firefox side.

Member

annevk commented Nov 3, 2017

Thanks, I'll fix that one too. I suspect it would be better for @cdumez et al if we fixed it here straight away given that there's still some (significant) delay if we land web-platform-tests changes from the Firefox side.

@annevk

This comment has been minimized.

Show comment
Hide comment
@annevk

annevk Nov 3, 2017

Member

Pushed a fix.

Member

annevk commented Nov 3, 2017

Pushed a fix.

@bzbarsky

Looks good, thank you!

"Object.keys() gives the right answer for cross-origin Window");
assert_array_equals(Object.getOwnPropertyNames(C.location).sort(),
whitelistedLocationPropNames,
"Object.getOwnPropertyNames() gives the right answer for cross-origin Location");
assert_array_equals(Object.keys(C.location).sort(),
whitelistedLocationPropNames,
assert_equals(Object.keys(C.location).length, 0,
"Object.keys() gives the right answer for cross-origin Location");

This comment has been minimized.

@bzbarsky

bzbarsky Nov 3, 2017

Contributor

Fix the indent here?

@bzbarsky

bzbarsky Nov 3, 2017

Contributor

Fix the indent here?

@wpt-pr-bot

This comment has been minimized.

Show comment
Hide comment
@wpt-pr-bot

wpt-pr-bot Nov 3, 2017

Collaborator

There are no owners for this pull request. Please reach out on W3C's irc server (irc.w3.org, port 6665) on channel #testing (web client) to get help with this. Thank you!

Collaborator

wpt-pr-bot commented Nov 3, 2017

There are no owners for this pull request. Please reach out on W3C's irc server (irc.w3.org, port 6665) on channel #testing (web client) to get help with this. Thank you!

@cdumez cdumez merged commit d7843d0 into master Nov 4, 2017

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

hubot pushed a commit to WebKit/webkit that referenced this pull request Nov 5, 2017

cdumez@apple.com
Index properties on cross origin Window objects should be enumerable
https://bugs.webkit.org/show_bug.cgi?id=179289

Reviewed by Darin Adler.

LayoutTests/imported/w3c:

Re-sync WPT test after:
- web-platform-tests/wpt#8045

Rebaseline a couple of WPT tests now that more checks are passing.

* web-platform-tests/html/browsers/origin/cross-origin-objects/cross-origin-objects-expected.txt:
* web-platform-tests/html/browsers/origin/cross-origin-objects/cross-origin-objects.html:
* web-platform-tests/html/browsers/the-window-object/window-indexed-properties-expected.txt:

Source/WebCore:

Index properties on cross origin Window objects should be enumerable:
- whatwg/html#3186
- web-platform-tests/wpt#8045

All exposed properties used to be enumerable but we had to revert this in
r224287 because it was not Web-compatible. The HTML specification has now
been updated so that only index properties are enumerable cross origin.

No new tests, rebaselined existing tests.

* bindings/js/JSDOMWindowCustom.cpp:
(WebCore::JSDOMWindow::getOwnPropertySlotByIndex):
(WebCore::JSDOMWindow::getOwnPropertyNames):

LayoutTests:

Update / rebaseline existing test to match new expected behavior.

* js/dom/getOwnPropertyDescriptor-expected.txt:
* js/resources/getOwnPropertyDescriptor.js:


git-svn-id: http://svn.webkit.org/repository/webkit/trunk@224464 268f45cc-cd09-0410-ab3c-d52691b4dbfc

@annevk annevk deleted the annevk/fewer-cross-origin-properties branch Nov 5, 2017

domenic added a commit to whatwg/html that referenced this pull request Nov 6, 2017

Enumerate fewer cross-origin properties
In 205659f we made all properties on
cross-origin objects enumerable, equivalent to their same-origin object
counterparts.

However, this turned out not be web-compatible. This makes them
non-enumerable again with the exception of array index property names,
which need to be enumerable.

Tests: web-platform-tests/wpt#8045

Fixes #3183.

moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this pull request Nov 7, 2017

Bug 1414292. Update to HTML spec changes for cross-origin object prop…
…erty enumerability. r=peterv

Updates to whatwg/html#3186

Includes the changes from web-platform-tests/wpt#8045

MozReview-Commit-ID: 5vEo1QGPufE

xeonchen pushed a commit to xeonchen/gecko that referenced this pull request Nov 7, 2017

Bug 1414292. Update to HTML spec changes for cross-origin object prop…
…erty enumerability. r=peterv

Updates to whatwg/html#3186

Includes the changes from web-platform-tests/wpt#8045

MozReview-Commit-ID: 5vEo1QGPufE

JerryShih pushed a commit to JerryShih/gecko-dev that referenced this pull request Nov 13, 2017

Bug 1414292. Update to HTML spec changes for cross-origin object prop…
…erty enumerability. r=peterv

Updates to whatwg/html#3186

Includes the changes from web-platform-tests/wpt#8045

MozReview-Commit-ID: 5vEo1QGPufE

aethanyc pushed a commit to aethanyc/gecko-dev that referenced this pull request Nov 16, 2017

Bug 1414292. Update to HTML spec changes for cross-origin object prop…
…erty enumerability. r=peterv

Updates to whatwg/html#3186

Includes the changes from web-platform-tests/wpt#8045

MozReview-Commit-ID: 5vEo1QGPufE
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment