Some WebIDL operations / attributes incorrectly use _current_ realm instead of _relevant_
Created attachment 439585 [details] Patch
This patch modifies the imported WPT tests. Please ensure that any changes on the tests (not coming from a WPT import) are exported to WPT. Please see https://trac.webkit.org/wiki/WPTExportProcess
<rdar://problem/83675608>
Comment on attachment 439585 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=439585&action=review > Source/WebCore/ChangeLog:22 > + Most of the remaining [CallWith=GlobalObject] instances are correctly used for > + converting JS arguments to WebIDL values; the rest, along with _current_ Document > + and ScriptExecutionContext, either match the spec or replacing them with _relevant_ > + global object is not directly observable (see condition #3). It would be great to have some guidance to others in the future that need to figure out which the correct document/globalObject,scriptExecutionContext is the right one to use (though I don't have a great idea where to put this guidance).
Perhaps also remaining the non-prefixed ones to use Current/Entry/etc prefixes would help, since people would feel they need to understand the concept before picking.
(In reply to Sam Weinig from comment #5) Thanks for review! > Perhaps also remaining the non-prefixed ones to use Current/Entry/etc > prefixes would help, since people would feel they need to understand the > concept before picking. Yeah, that is the plan. Having _current_ by default, for non-prefixed values, is prone to introducing incompatibilities. For now I will add a spec link to both IDLAttributes.json and CodeGeneratorJS.pm; WHATWG editors did a great job explaining the concepts.
(In reply to Alexey Shvayka from comment #6) > (In reply to Sam Weinig from comment #5) > > Thanks for review! > > > Perhaps also remaining the non-prefixed ones to use Current/Entry/etc > > prefixes would help, since people would feel they need to understand the > > concept before picking. > > Yeah, that is the plan. Having _current_ by default, for non-prefixed > values, is prone to introducing incompatibilities. > > For now I will add a spec link to both IDLAttributes.json and > CodeGeneratorJS.pm; WHATWG editors did a great job explaining the concepts. Sounds great. (The more we can use spec names in our code, the better in my book. I would love to see more done in this area, especially in the area of JSC functions that WebCore bindings have to interact with).
Created attachment 446847 [details] Patch for landing
(In reply to Alexey Shvayka from comment #8) > Created attachment 446847 [details] > Patch for landing Also added bindings tests for newly added Relevant* values.
Committed r286895 (245123@main): <https://commits.webkit.org/245123@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 446847 [details].
(In reply to Sam Weinig from comment #5) > Perhaps also remaining the non-prefixed ones to use Current/Entry/etc > prefixes would help, since people would feel they need to understand the > concept before picking. Hey Sam, that change is now up for available for review: https://bugs.webkit.org/show_bug.cgi?id=236137. Thanks!