1127703 - Support iteration on FormData

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect)

Comment 1

[Andrea Marchesini [:baku]]

Attachment: iterable2.patch

Comment 2

[Andrea Marchesini [:baku]]

Attachment: iterable2.patch

Comment 3

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 8675710 [details] [diff] [review]
iterable2.patch

>+    if (tmp->mFormData[i].value.IsFile()) {
>+      ImplCycleCollectionUnlink(tmp->mFormData[i].value.GetAsFile());

How about:

  ImplCycleCollectionUnlink(tmp->mFormData[i].value)

?

And similar in traverse.  We codegen implementations of ImplCycleCollectionTraverse/ImplCycleCollectionUnlink for owning unions, precisely so you don't have to dig around inside them (especially important when they can have multiple refptr types).

>+      ImplCycleCollectionTraverse(cb,tmp->mFormData[i].value.GetAsFile(),

Missing space before "tmp".

>+    is(key.value, v.toString(), "Correct Key iterator: " + v.toString());
>+    ok(key.done, "key.done is false");

Should be ok(!key.done, "key.done should be false"), no?

>+    ok(value.done, "value.done is false");
>+    ok(entry.done, "entry.done is false");

And here.

>+++ b/xpcom/base/OwningNonNull.h

I guess it doesn't really hurt to add this, but I don't think it's really needed for this patch either...

r=me with the above fixed.

Comment 4

[Andrea Marchesini [:baku]]

Attachment: 268332.diff

Comment 5

[Andrea Marchesini [:baku]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/736f0981762c

Comment 6

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

Backed out for build bustage: https://treeherder.mozilla.org/logviewer.html#?job_id=15869511&repo=mozilla-inbound

https://hg.mozilla.org/integration/mozilla-inbound/rev/5f2853528bbf

Comment 7

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/c159526f9145

Comment 8

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

Backed out again in https://hg.mozilla.org/integration/mozilla-inbound/rev/d273e971464c for test_formsubmission failures:

https://treeherder.mozilla.org/logviewer.html#?job_id=15880296&repo=mozilla-inbound

Comment 9

[Andrea Marchesini [:baku]]

Attachment: iterable2.patch

Comment 10

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/512b90a0f35f

Comment 11

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/512b90a0f35f

Comment 12

[Jean-Yves Perrier [:teoli]]

Created:
https://developer.mozilla.org/en-US/docs/Web/API/FormData/values
https://developer.mozilla.org/en-US/docs/Web/API/FormData/keys
https://developer.mozilla.org/en-US/docs/Web/API/FormData/entries

I hope I have not forgotten a newly created method (first time we are documenting an iterator<> webidl value)

Updated:
https://developer.mozilla.org/en-US/docs/Web/API/FormData
and
https://developer.mozilla.org/en-US/Firefox/Releases/44#Miscellaneous

Comment 13

[Boris Zbarsky [:bzbarsky]]

Jean-Yves, it's not clear to me what the "In most cases, the use of the for...of statement is preferrable to access the key/value pairs." bits in those docs are meant to suggest/imply.

Are they trying to say that it's better to just for..of on the FormData object itself?  Or on the iterators?  Something else?

Similarly, it's not clear what "objects implementing this interface can be used in a for...of structure" means in <https://developer.mozilla.org/en-US/docs/Web/API/FormData>.  Could probably use an example usage.  Just like it might be good to have examples in the values/keys/entries pages of using for-of with the iterators there ("for (var key of formData.keys()) { /* etc */ }").

There will also be a forEach method here once bug 1216751 is fixed, but it's not there yet.

Comment 14

[Jean-Yves Perrier [:teoli]]

Thx. ni/ myself so I don't forget to come back to this (doing too many things in // right now ;-) )

Comment 15

[Jean-Yves Perrier [:teoli]]

So it happens that I didn't understood that I could directly use an iterator in for..of (which is rather obvious now, silly me!).

I fixed the three methods pages removing the sentence (that made no sense) and fixed the example to use for...of.

On FormData, I modified the sentence to explain that 'for (var pair of myFD)' is equivalent to 'for (var pair of myFD.entries())'.

I also added dev-doc-needed and a comment to bug 1216751 so that we don't forget add forEach() when it lands.

I updated our meta-documentation about how to document iterator<>: https://developer.mozilla.org/en-US/docs/MDN/Contribute/Howto/Write_an_API_reference/Information_contained_in_a_WebIDL_file#Pair_iterator

Comment 16

[Boris Zbarsky [:bzbarsky]]

Thank you!  That meta-documentation is pretty awesome, by the way.