Bug 942631 finally allows us to have real arrays \o/

Need deps before tests pass.

Comment on attachment 8343334 [details] [diff] [review] Use sequences for the array attributes I did some tests using find() and 1500 contacts and this makes us ~8% faster, probably due to the manual validation with proxies that can be removed now.

Hmm. save() might still need to do some sort of validation. In particular, unfortunately nothing guarantees that aContact.tel will only contain vanilla JS reflections of ContactTelField. It could have whatever the page stuck in there, right?

(In reply to Boris Zbarsky [:bz] from comment #3) > Hmm. > > save() might still need to do some sort of validation. In particular, > unfortunately nothing guarantees that aContact.tel will only contain vanilla > JS reflections of ContactTelField. I hope not! Right now if I try to do something like this: var c = new mozContact({tel: []}); c.tel.push({aeiou: "abcde"}); I get an empty object in save(). As long as there are no cross-compartment shenanigans, I think this is fine. There's only so much hand-holding we can do with the API users…

Reuben and I talked that part over. We do in fact need to do validation, but this will do it fine: try { for (let field of PROPERTIES) { aContact[field] = aContact[field]; } } catch (e) { throw this._window.DOMError(e.name, e.message); } with some comments explaining why the "no-op" assignment and why the weird throwing behavior.

Incorporates the changes to save() needed to make sure we validate everything in the sequences before we save the contact.

Comment on attachment 8349615 [details] [diff] [review] Use sequences for the array attributes, v2 r=me, though I only skimmed the test changes.

Comment on attachment 8349615 [details] [diff] [review] Use sequences for the array attributes, v2 Thanks! Gregor, can you take a look? Specifically any unwanted changes to the behavior? The test changes cover the obvious ones. Something new that we may want to add a test for is the fact that save() can now throw if you pass a mozContact with invalid data.

(In reply to :Reuben Morais [slow to respond until Jan 10] from comment #9) > Comment on attachment 8349615 [details] [diff] [review] > Use sequences for the array attributes, v2 > > Thanks! Gregor, can you take a look? Specifically any unwanted changes to > the behavior? The test changes cover the obvious ones. Something new that we > may want to add a test for is the fact that save() can now throw if you pass > a mozContact with invalid data. Hm thats a good question. I guess it should be similar to other APIs we design. Ehsan, Jonas, any comments here? But if it throws we should at least add a test case for it.

I can't think of another difference except for save (and also remove) to start throwing if handed off an invalid object.

Throwing if you attempt to save something that isn't a valid contact sounds fine to me. As long as we are backwards compatible with old code. I think we should even throw if the page attempts to use a simple string value where an array of strings is what the API normally uses. Though it's probably too late to make such a change now.

Ugh. I forgot I added a test.