Title: [264948] trunk
- Revision
- 264948
- Author
- cdu...@apple.com
- Date
- 2020-07-27 15:06:31 -0700 (Mon, 27 Jul 2020)
Log Message
thisValue is not always set correctly when calling JS callbacks
https://bugs.webkit.org/show_bug.cgi?id=214847
Reviewed by Darin Adler.
LayoutTests/imported/w3c:
Import layout test coverage from upstream WPT. We were failing this new subtest
before my fix.
* web-platform-tests/dom/traversal/TreeWalker-acceptNode-filter-expected.txt:
* web-platform-tests/dom/traversal/TreeWalker-acceptNode-filter.html:
Source/WebCore:
thisValue was not always set correctly when calling JS callbacks.
This was causing us to fail the last subtest on:
http://w3c-test.org/dom/traversal/TreeWalker-acceptNode-filter.html
The specification for TreeWalker is here:
- https://dom.spec.whatwg.org/#concept-node-filter
Step 6 refers to [call a user object’s operation] in the WebIDL specification:
- https://heycam.github.io/webidl/#call-a-user-objects-operation
Step 10.5 says:
"Set thisArg to O (overriding the provided value)."
We were missing this step in our implementation so thisValue ended up being undefined
(as per earlier step 2).
No new tests, resync'd existing test.
* bindings/js/JSCallbackData.cpp:
(WebCore::JSCallbackData::invokeCallback):
Modified Paths
Diff
Modified: trunk/LayoutTests/imported/w3c/ChangeLog (264947 => 264948)
--- trunk/LayoutTests/imported/w3c/ChangeLog 2020-07-27 22:02:03 UTC (rev 264947)
+++ trunk/LayoutTests/imported/w3c/ChangeLog 2020-07-27 22:06:31 UTC (rev 264948)
@@ -1,3 +1,16 @@
+2020-07-27 Chris Dumez <cdu...@apple.com>
+
+ thisValue is not always set correctly when calling JS callbacks
+ https://bugs.webkit.org/show_bug.cgi?id=214847
+
+ Reviewed by Darin Adler.
+
+ Import layout test coverage from upstream WPT. We were failing this new subtest
+ before my fix.
+
+ * web-platform-tests/dom/traversal/TreeWalker-acceptNode-filter-expected.txt:
+ * web-platform-tests/dom/traversal/TreeWalker-acceptNode-filter.html:
+
2020-07-27 Clark Wang <clark_w...@apple.com>
Added Constructor method to OscillatorNode
Modified: trunk/LayoutTests/imported/w3c/web-platform-tests/dom/traversal/TreeWalker-acceptNode-filter-expected.txt (264947 => 264948)
--- trunk/LayoutTests/imported/w3c/web-platform-tests/dom/traversal/TreeWalker-acceptNode-filter-expected.txt 2020-07-27 22:02:03 UTC (rev 264947)
+++ trunk/LayoutTests/imported/w3c/web-platform-tests/dom/traversal/TreeWalker-acceptNode-filter-expected.txt 2020-07-27 22:06:31 UTC (rev 264948)
@@ -10,4 +10,5 @@
PASS rethrows errors when getting `acceptNode`
PASS performs `Get` on every traverse
PASS Testing with filter object that throws
+PASS Testing with filter object: this value and `node` argument
Test JS objects as NodeFilters
Modified: trunk/LayoutTests/imported/w3c/web-platform-tests/dom/traversal/TreeWalker-acceptNode-filter.html (264947 => 264948)
--- trunk/LayoutTests/imported/w3c/web-platform-tests/dom/traversal/TreeWalker-acceptNode-filter.html 2020-07-27 22:02:03 UTC (rev 264947)
+++ trunk/LayoutTests/imported/w3c/web-platform-tests/dom/traversal/TreeWalker-acceptNode-filter.html 2020-07-27 22:06:31 UTC (rev 264948)
@@ -172,6 +172,24 @@
assert_node(walker.currentNode, { type: Element, id: 'root' });
}, 'Testing with filter object that throws');
+test(() =>
+{
+ let thisValue, nodeArgID;
+ const filter = {
+ acceptNode(node) {
+ thisValue = this;
+ nodeArgID = node.id;
+ return NodeFilter.FILTER_ACCEPT;
+ },
+ };
+
+ const walker = document.createTreeWalker(testElement, NodeFilter.SHOW_ELEMENT, filter);
+ walker.nextNode();
+
+ assert_equals(thisValue, filter);
+ assert_equals(nodeArgID, 'A1');
+}, 'Testing with filter object: this value and `node` argument');
+
</script>
</body>
</html>
Modified: trunk/Source/WebCore/ChangeLog (264947 => 264948)
--- trunk/Source/WebCore/ChangeLog 2020-07-27 22:02:03 UTC (rev 264947)
+++ trunk/Source/WebCore/ChangeLog 2020-07-27 22:06:31 UTC (rev 264948)
@@ -1,3 +1,32 @@
+2020-07-27 Chris Dumez <cdu...@apple.com>
+
+ thisValue is not always set correctly when calling JS callbacks
+ https://bugs.webkit.org/show_bug.cgi?id=214847
+
+ Reviewed by Darin Adler.
+
+ thisValue was not always set correctly when calling JS callbacks.
+
+ This was causing us to fail the last subtest on:
+ http://w3c-test.org/dom/traversal/TreeWalker-acceptNode-filter.html
+
+ The specification for TreeWalker is here:
+ - https://dom.spec.whatwg.org/#concept-node-filter
+
+ Step 6 refers to [call a user object’s operation] in the WebIDL specification:
+ - https://heycam.github.io/webidl/#call-a-user-objects-operation
+
+ Step 10.5 says:
+ "Set thisArg to O (overriding the provided value)."
+
+ We were missing this step in our implementation so thisValue ended up being undefined
+ (as per earlier step 2).
+
+ No new tests, resync'd existing test.
+
+ * bindings/js/JSCallbackData.cpp:
+ (WebCore::JSCallbackData::invokeCallback):
+
2020-07-27 Sihui Liu <sihui_...@appe.com>
Text manipulation should not extract non-breaking spaces
Modified: trunk/Source/WebCore/bindings/js/JSCallbackData.cpp (264947 => 264948)
--- trunk/Source/WebCore/bindings/js/JSCallbackData.cpp 2020-07-27 22:02:03 UTC (rev 264947)
+++ trunk/Source/WebCore/bindings/js/JSCallbackData.cpp 2020-07-27 22:06:31 UTC (rev 264948)
@@ -38,6 +38,7 @@
namespace WebCore {
using namespace JSC;
+// https://heycam.github.io/webidl/#call-a-user-objects-operation
JSValue JSCallbackData::invokeCallback(JSDOMGlobalObject& globalObject, JSObject* callback, JSValue thisValue, MarkedArgumentBuffer& args, CallbackType method, PropertyName functionName, NakedPtr<JSC::Exception>& returnedException)
{
ASSERT(callback);
@@ -72,6 +73,8 @@
returnedException = JSC::Exception::create(vm, createTypeError(lexicalGlobalObject));
return JSValue();
}
+
+ thisValue = callback;
}
ASSERT(!function.isEmpty());
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes
