Bug 1734244 Comment 8 Edit History

Note: The actual edited comment in the bug view page will always show the original commenter’s name and original timestamp.

Original comment by
on 
(In reply to Tom Schuster [:evilpie] from comment #7)
> Peter, like you said `GetNextIterationResult` doesn't have a `JSContext` argument anymore, but I think we already use `AutoJSAPI` for something similar in other places.

I'll take a look, if it's not ok then I can write a patch to pass in a JSContext.

> There is also one failure that I think is related to the bindings implementation
> > Async iterator instances should have the correct list of properties
> 
> It seems like the `length` property of the `return` method is 0, but should should be 1.

Yeah, it's not clear to me that that's a valid test. AFAICT the value argument for return methods is supposed to be optional, and length shouldn't count optional arguments. I'll probably file a WebIDL spec bug to clarify this.
Revision 1 by
on 
(In reply to Tom Schuster [:evilpie] from comment #7)
> Peter, like you said `GetNextIterationResult` doesn't have a `JSContext` argument anymore, but I think we already use `AutoJSAPI` for something similar in other places.

I'll take a look, if it's not ok then I can write a patch to pass in a JSContext.

> There is also one failure that I think is related to the bindings implementation
> > Async iterator instances should have the correct list of properties
> 
> It seems like the `length` property of the `return` method is 0, but should should be 1.

Yeah, it's not clear to me that that's a valid test. AFAICT the `value` argument for `return` methods is supposed to be optional, and `length` shouldn't count optional arguments. I'll probably file a WebIDL spec bug to clarify this.

Back to Bug 1734244 Comment 8