diff options
author | bors-servo <lbergstrom+bors@mozilla.com> | 2020-01-10 15:18:10 -0500 |
---|---|---|
committer | GitHub <noreply@github.com> | 2020-01-10 15:18:10 -0500 |
commit | fdfc840baca427f58daaa251100c3940256e86c5 (patch) | |
tree | df81b7e1e8df96086926078a9c8b4b91161be558 | |
parent | 2aae85b0a0b61350fe946f6b75ed31cd1d5ea30f (diff) | |
parent | 111ede9c77a41ab353f12683a62cd4b4dabb65da (diff) | |
download | servo-fdfc840baca427f58daaa251100c3940256e86c5.tar.gz servo-fdfc840baca427f58daaa251100c3940256e86c5.zip |
Auto merge of #25446 - pshaughn:fixme11868, r=jdm
Make getOwnPropertyDescriptor hold the correct value for indexed/named properties
This is towards #25036 and #25415; it could have far-reaching implications so I want to test it in isolation and see the results on the full test suite.
A few lines of code had a FIXME(#11868) despite that issue being closed, and looking for the pattern that was marked that way, I found one other unmarked instance of it. It doesn't immediately crash, and maybe it will pass some tests or fail some tests in informative ways.
EDIT: After adding an overlooked extended attribute to HTMLFormElement, this works very well indeed and seems to be worth merging!
---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `___` with appropriate data: -->
- [X] `./mach build -d` does not report any errors
- [X] `./mach test-tidy` does not report any errors
- [X] These changes fix #25036
<!-- Either: -->
- [X] There are tests for these changes OR
- [ ] These changes do not require tests because ___
<!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.-->
<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->
8 files changed, 6 insertions, 30 deletions
diff --git a/components/script/dom/bindings/codegen/CodegenRust.py b/components/script/dom/bindings/codegen/CodegenRust.py index e0a5df68882..d79bc073e76 100644 --- a/components/script/dom/bindings/codegen/CodegenRust.py +++ b/components/script/dom/bindings/codegen/CodegenRust.py @@ -5147,8 +5147,7 @@ class CGDOMJSProxyHandler_getOwnPropertyDescriptor(CGAbstractExternMethod): attrs = "JSPROP_ENUMERATE" if self.descriptor.operations['IndexedSetter'] is None: attrs += " | JSPROP_READONLY" - # FIXME(#11868) Should assign to desc.value, desc.get() is a copy. - fillDescriptor = ("desc.get().value = result_root.get();\n" + fillDescriptor = ("desc.value = result_root.get();\n" "fill_property_descriptor(MutableHandle::from_raw(desc), proxy.get(), (%s) as u32);\n" "return true;" % attrs) templateValues = { @@ -5173,8 +5172,7 @@ class CGDOMJSProxyHandler_getOwnPropertyDescriptor(CGAbstractExternMethod): attrs = " | ".join(attrs) else: attrs = "0" - # FIXME(#11868) Should assign to desc.value, desc.get() is a copy. - fillDescriptor = ("desc.get().value = result_root.get();\n" + fillDescriptor = ("desc.value = result_root.get();\n" "fill_property_descriptor(MutableHandle::from_raw(desc), proxy.get(), (%s) as u32);\n" "return true;" % attrs) templateValues = { @@ -5221,7 +5219,7 @@ if !expando.is_null() { } } """ + namedGet + """\ -desc.get().obj = ptr::null_mut(); +desc.obj = ptr::null_mut(); return true;""" def definition_body(self): diff --git a/components/script/dom/webidls/HTMLFormElement.webidl b/components/script/dom/webidls/HTMLFormElement.webidl index 13ffbf4dfe8..744f6d84ca9 100644 --- a/components/script/dom/webidls/HTMLFormElement.webidl +++ b/components/script/dom/webidls/HTMLFormElement.webidl @@ -3,7 +3,7 @@ * file, You can obtain one at https://mozilla.org/MPL/2.0/. */ // https://html.spec.whatwg.org/multipage/#htmlformelement -[Exposed=Window] +[Exposed=Window, LegacyUnenumerableNamedProperties] interface HTMLFormElement : HTMLElement { [HTMLConstructor] constructor(); diff --git a/components/script/dom/webidls/Window.webidl b/components/script/dom/webidls/Window.webidl index f2cdeeb18a5..afd5183de33 100644 --- a/components/script/dom/webidls/Window.webidl +++ b/components/script/dom/webidls/Window.webidl @@ -3,7 +3,7 @@ * file, You can obtain one at https://mozilla.org/MPL/2.0/. */ // https://html.spec.whatwg.org/multipage/#window -[Global=Window, Exposed=Window] +[Global=Window, Exposed=Window /*, LegacyUnenumerableNamedProperties */] /*sealed*/ interface Window : GlobalScope { // the current browsing context [Unforgeable] readonly attribute WindowProxy window; diff --git a/components/script/dom/windowproxy.rs b/components/script/dom/windowproxy.rs index 7fea1728c12..6aa0b9244cf 100644 --- a/components/script/dom/windowproxy.rs +++ b/components/script/dom/windowproxy.rs @@ -822,8 +822,7 @@ unsafe extern "C" fn getOwnPropertyDescriptor( assert!(desc.obj.is_null() || desc.obj == target.get()); if desc.obj == target.get() { - // FIXME(#11868) Should assign to desc.obj, desc.get() is a copy. - desc.get().obj = proxy.get(); + desc.obj = proxy.get(); } true diff --git a/tests/wpt/metadata/WebIDL/ecmascript-binding/legacy-platform-object.html.ini b/tests/wpt/metadata/WebIDL/ecmascript-binding/legacy-platform-object.html.ini index a2250466733..0ba8ad10aaa 100644 --- a/tests/wpt/metadata/WebIDL/ecmascript-binding/legacy-platform-object.html.ini +++ b/tests/wpt/metadata/WebIDL/ecmascript-binding/legacy-platform-object.html.ini @@ -1,14 +1,5 @@ [legacy-platform-object.html] type: testharness - [[[GetOwnProperty\]\] with getters and no setters] - expected: FAIL - - [[[GetOwnProperty\]\] with named property getters and setters] - expected: FAIL - - [[[GetOwnProperty\]\] with indexed property getters and setters] - expected: FAIL - [Test [[DefineOwnProperty\]\] with no indexed property setter support.] expected: FAIL diff --git a/tests/wpt/metadata/dom/collections/HTMLCollection-supported-property-indices.html.ini b/tests/wpt/metadata/dom/collections/HTMLCollection-supported-property-indices.html.ini index 4bf756bec94..16ea183e3c0 100644 --- a/tests/wpt/metadata/dom/collections/HTMLCollection-supported-property-indices.html.ini +++ b/tests/wpt/metadata/dom/collections/HTMLCollection-supported-property-indices.html.ini @@ -6,6 +6,3 @@ [Trying to set an expando with an indexed property name past the end of the list] expected: FAIL - [Trying to delete an indexed property name should never work] - expected: FAIL - diff --git a/tests/wpt/metadata/html/dom/elements/global-attributes/dataset-binding.window.js.ini b/tests/wpt/metadata/html/dom/elements/global-attributes/dataset-binding.window.js.ini index 19996050bff..1fcfb3b5887 100644 --- a/tests/wpt/metadata/html/dom/elements/global-attributes/dataset-binding.window.js.ini +++ b/tests/wpt/metadata/html/dom/elements/global-attributes/dataset-binding.window.js.ini @@ -5,9 +5,3 @@ [Setting property for key 9 with accessor property on prototype] expected: FAIL - [Getting property descriptor for key 9] - expected: FAIL - - [Getting property descriptor for key x] - expected: FAIL - diff --git a/tests/wpt/metadata/html/semantics/forms/the-form-element/form-nameditem.html.ini b/tests/wpt/metadata/html/semantics/forms/the-form-element/form-nameditem.html.ini index 2fc5417441d..b33041d0dc6 100644 --- a/tests/wpt/metadata/html/semantics/forms/the-form-element/form-nameditem.html.ini +++ b/tests/wpt/metadata/html/semantics/forms/the-form-element/form-nameditem.html.ini @@ -1,8 +1,5 @@ [form-nameditem.html] type: testharness - [Name for a single element should work] - expected: FAIL - [All listed elements except input type=image should be present in the form] expected: FAIL |