Fix collection serialization and support collection types found in the wild #1340
Conversation
The way the internal JSON clone algorithm is written it will not be possible for an implementation to hit all variants. For example, we return if the value is an instance of an Object before we check if it has a callable toJSON property. Another change in this patch is that every JS object can be serialised and returned, so it does not make sense to return an error at the end, since everything in JS is an object.
Previously collections such as Array, HTMLCollection, and NodeList were copied similarly to Objects. This would cause them to never be expanded into arrays. This changes it so that we look at each of the items on their iterable object and serialise each of the items.
…ollections WebDriver previously only handled serialisation of Array, NodeList, and HTMLCollection. Implementations in the wild handle further types, such as Arguments, FileList, HTMLAllCollection, HTMLFormControlsCollection, and HTMLOptionsCollection.
|
This seems a bit nonsensical? We now have "A collection is an Object that implements the Iterable interface, and whose: […] type is HTMLAllCollection". HTMLAllCollection isn't an interface with an iterable declaration, so instances are still not collections. (I haven't gone through all the types listed, so there might be others which similarly don't have iterable declarations.) |
|
Also: what's a "type"? |
|
Yes, thanks for pointing those things out. There are some rough edges here. The effect we want to achieve with this PR is that those things we define as “collection types” should be expanded to an array of items, for which we invoke the serialisation algorithm again on each item. In Marionette we implement this in JS using HTML uses a narrower definition of what a collection is because it limits them to things deriving from NodeList if I remember correctly. In particular this excludes the Arguments object, but more importantly, there is not a centralised list of collections in HTML or DOM, and this is an attempt an enumerating those, plus adding Arguments into the mix. “type” probably should be replace with “an instance of”. |
WebDriver’s JSON serialisation algorithm doesn’t handle all the collection types handled by implementations found in the wild. These are:
Whilst adding these I also discovered that the algorithm didn’t really inspect the interior of array-like collections at all, so the PR also fixes that. This is effectively mirroring how geckodriver handles JSON currently. See each commit message for more detailed information, since I also fixed a few other minor bugs along the way.
Tests for this can be found in https://github.com/web-platform-tests/wpt/blob/master/webdriver/tests/execute_script/collections.py.
This was discovered whilst reviewing web-platform-tests/wpt#13880.