Skip to content

Enumerate fewer cross-origin properties - #8045

Merged
cdumez merged 2 commits into
masterfrom
annevk/fewer-cross-origin-properties
Nov 4, 2017
Merged

cdumez merged 2 commits into
masterfrom
annevk/fewer-cross-origin-properties

Conversation

@annevk

@annevk annevk commented Nov 2, 2017 •

Copy link
Copy Markdown
Member

See whatwg/html#3186 for context.

@annevk

annevk commented Nov 2, 2017

Copy link
Copy Markdown
Member Author

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.

@ghost

ghost commented Nov 2, 2017 •

Copy link
Copy Markdown

Build PASSED

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

View more information about this build on:

@bzbarsky

bzbarsky commented Nov 2, 2017

Copy link
Copy Markdown
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...

@annevk

annevk commented Nov 3, 2017

Copy link
Copy Markdown
Member Author

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

annevk commented Nov 3, 2017

Copy link
Copy Markdown
Member Author

Pushed a fix.

@bzbarsky bzbarsky left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Looks good, thank you!

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");

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Fix the indent here?

@wpt-pr-bot

Copy link
Copy Markdown
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!

@cdumez
cdumez merged commit d7843d0 into master Nov 4, 2017
hubot pushed a commit to WebKit/WebKit-http that referenced this pull request Nov 5, 2017
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 November 5, 2017 17:00
domenic pushed a commit to whatwg/html that referenced this pull request Nov 6, 2017
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
…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-cinnabar that referenced this pull request Nov 7, 2017
…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
…erty enumerability. r=peterv

Updates to whatwg/html#3186

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

MozReview-Commit-ID: 5vEo1QGPufE
alice pushed a commit to alice/html that referenced this pull request Jan 8, 2019
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 whatwg#3183.
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified-and-comments-removed that referenced this pull request Oct 2, 2019
…erty enumerability. r=peterv

Updates to whatwg/html#3186

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

MozReview-Commit-ID: 5vEo1QGPufE

UltraBlame original commit: 2a5a80d284b5ef958298cf515867ffbe420478c0
gecko-dev-updater pushed a commit to marco-c/gecko-dev-comments-removed that referenced this pull request Oct 2, 2019
…erty enumerability. r=peterv

Updates to whatwg/html#3186

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

MozReview-Commit-ID: 5vEo1QGPufE

UltraBlame original commit: 2a5a80d284b5ef958298cf515867ffbe420478c0
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified that referenced this pull request Oct 2, 2019
…erty enumerability. r=peterv

Updates to whatwg/html#3186

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

MozReview-Commit-ID: 5vEo1QGPufE

UltraBlame original commit: 2a5a80d284b5ef958298cf515867ffbe420478c0
ryanhaddad pushed a commit to WebKit/WebKit that referenced this pull request Dec 22, 2020
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:


Canonical link: https://commits.webkit.org/195389@main
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@224464 268f45cc-cd09-0410-ab3c-d52691b4dbfc
bhearsum pushed a commit to mozilla-releng/staging-firefox that referenced this pull request May 1, 2025
…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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants