We must implement reorder in Bookmarks.jsm so we can finally replace usage of setItemIndex.

We're dropping it from this iteration for now and will pick it up again later.

/r/1523 - Bug 1081108 - Implement reorder in Bookmarks.jsm. r=Mano Pull down this commit: hg pull review -r 813c0839f7c141447e352f3ed3f6e22fc0aa250a

https://reviewboard.mozilla.org/r/1523/#review1019 ::: toolkit/components/places/Bookmarks.jsm (Diff revision 1) > + throw new Error("No folder found for the provided GUID."); This sounds as if the guid isn't found. The error message should reflect that the pointed item isn't a folder. ::: toolkit/components/places/Bookmarks.jsm (Diff revision 1) > + // Reorder the children array according to the wanted order, provided s/wanted/specified ::: toolkit/components/places/Bookmarks.jsm (Diff revision 1) > + return (i != -1 && j != -1 && i < j) || (i != -1 && j == -1) ? -1 : 1; So, given a folder that looks like this pre-sort: [a, b, d, c] and a [a, b] reorder call, does this ensure that c will still follow d? ::: toolkit/components/places/Bookmarks.jsm (Diff revision 1) > + // Update the position now. If any unknown GUID should have been Either "Should any unknown guid have been inserted meanwhile" or "If any unknown guid have been inserted meanwhile" ::: toolkit/components/places/Bookmarks.jsm (Diff revision 1) > + // To do the update in one step, we build a VALUES (guid, position) a single step/query ::: toolkit/components/places/Bookmarks.jsm (Diff revision 1) > + i, child.type, If you use child.index here (rather than i), you can use a for-of loop. ::: toolkit/components/places/Bookmarks.jsm (Diff revision 1) > + JOIN sorting b ON b.p <= a.p trailing space ::: toolkit/components/places/Bookmarks.jsm (Diff revision 1) > + // table. We use count() in the sorting table to avoid skipping values We then ::: toolkit/components/places/tests/bookmarks/test_bookmarks_notifications.js (Diff revision 1) > + }, Remove , ::: toolkit/components/places/tests/bookmarks/test_bookmarks_notifications.js (Diff revision 1) > + } sorted = [for (bm of bookmarks] yield PlacesUtils.bookmarks.insert(b)]

https://reviewboard.mozilla.org/r/1523/#review1021 > If you use child.index here (rather than i), you can use a for-of loop. child.index is NOT updated... I could update it, with another loop, but I didn't so far cause we are not returning them. > So, given a folder that looks like this pre-sort: > [a, b, d, c] and a [a, b] reorder call, does this ensure that c will still follow d? nope, ordering of not passed-in guids is unspecified, retaining it would be more complex (I'm not even sure if it is possible with this implmentation)

I can probably try to retain the original order by setting position to -position rather than -1. I have some tentative queries that seem to be working. let me see.

https://reviewboard.mozilla.org/r/1523/#review1025 > This sounds as if the guid isn't found. The error message should reflect that the pointed item isn't a folder. It could also not be found (!parent || parent.type != this.TYPE_FOLDER) so the message is more generic on purpose. > child.index is NOT updated... I could update it, with another loop, but I didn't so far cause we are not returning them. Moreover child.index is the old index and we need both old and new index for the notification.

/r/1523 - Bug 1081108 - Implement reorder in Bookmarks.jsm. r=Mano Pull down this commit: hg pull review -r 06226488a19a041b50fa49aff220c460c9c3d2c6