Repository navigation
Enumerate less cross-origin properties - #3186
Conversation
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 unenumerable again with the exception of array index property names, which need to be enumerable. Tests: ... Fixes #3183.
|
(That should probably read "Enumerate fewer ..." looking at it again.) |
See whatwg/html#3186 for context.
|
Tests are at web-platform-tests/wpt#8045. Review on those and this PR appreciated @bzbarsky and @cdumez. Given that Firefox and Safari are actively involved, we'll only need to update these bugs once this is merged:
(Brief testing shows at least Chrome still needing changes as array index property names are not enumerable currently.) |
|
@annevk Thank you for fixing this. Is it at all possible to see what the rendered version looks like? Reviewing the diff is a bit of a pain, especially with github refusing to expand the context... |
|
I flipped the third and fourth instance of [[Enumerable]] on https://html.spec.whatwg.org/multipage/browsers.html and the second instance on https://html.spec.whatwg.org/multipage/window-object.html. Let me know if that helps. If it doesn't I can see about generating a copy (I'm still transitioning setups so I don't have everything up and running locally). |
So in https://html.spec.whatwg.org/multipage/browsers.html#crossorigingetownpropertyhelper-(-o,-p-) step 3, right?
So https://html.spec.whatwg.org/multipage/window-object.html#windowproxy-getownproperty steps 6? |
bzbarsky
left a comment
There was a problem hiding this comment.
Looks good assuming I got the right steps in #3186 (comment)
|
Should we add a note or example about how this inconsistency is required for web compat? Or at least a HTML comment so we don't forget and try to "fix" things again in 5 years? Happy to do that as a follow-up. |
|
If you have a suggested place I'm happy to add something. Or you can push a fixup. I don't think we need to land this now, but somewhere within the next week would be good. |
* Enumerate fewer cross-origin properties See whatwg/html#3186 for context. * adjust Object.keys() subtests as well
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
|
Added a note; let me know what you think. |
annevk
left a comment
There was a problem hiding this comment.
LGTM. I cannot approve since I submitted the original PR. The only wonder I have is whether we should combine the two notes at the end of CrossOriginGetOwnPropertyHelper into a single <div class=note>.
|
I think separate notes for separate topics is slightly nicer. So, I'll go ahead and merge this. |
|
Updated the Chrome and Edge bugs. |
|
Thank you very much for the quick response on this! |
…erty enumerability. r=peterv Updates to whatwg/html#3186 Includes the changes from web-platform-tests/wpt#8045 MozReview-Commit-ID: 5vEo1QGPufE
…erty enumerability. r=peterv Updates to whatwg/html#3186 Includes the changes from web-platform-tests/wpt#8045 MozReview-Commit-ID: 5vEo1QGPufE
…erty enumerability. r=peterv Updates to whatwg/html#3186 Includes the changes from web-platform-tests/wpt#8045 MozReview-Commit-ID: 5vEo1QGPufE
…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
…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
…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
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
…erty enumerability. r=peterv Updates to whatwg/html#3186 Includes the changes from web-platform-tests/wpt#8045 MozReview-Commit-ID: 5vEo1QGPufE
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 unenumerable again with the exception of array index property names, which need to be enumerable.
Tests: ...
Fixes #3183.