Move-to-the-end is a common use case that's currently unsupported by PlacesUtils.bookmarks.update (unless you pass something like MAX_INT for index). This is necessary for reimplementing PlacesTransactions.Move using the update API.

I also fixed a bug in update: moving an item to a "too high" index within the same folder was treated as appending. Note that while the function "internals" support update( index: defualt ) without specifying parentGuid, the input validation disallows that. We may want to revise this decision later.

Comment on attachment 8527592 [details] [diff] [review] patch Review of attachment 8527592 [details] [diff] [review]: ----------------------------------------------------------------- ::: toolkit/components/places/Bookmarks.jsm @@ +306,5 @@ > + if (!parent || parent.guid == item.parentGuid) { // Same parent. > + // If at this point we don't have a parent yet, we are moving into > + // the same container. Thus we know it exists. > + if (!parent) > + parent = yield fetchBookmark({ guid: item.parentGuid }); what about // current comment if (!parent) parent = if (updateInfo.index >= parent._childCount || updateInfo.index == this.DEFAULT_INDEX) { updateInfo.index = parent._childCount; } // Fix the index when moving into the same container. if (parent.guid == item.parentGuid) updateInfo.index--; ::: toolkit/components/places/tests/bookmarks/test_bookmarks_update.js @@ +422,5 @@ > + let sep_3 = yield PlacesUtils.bookmarks.insert({ parentGuid: folder_a.guid, > + type: PlacesUtils.bookmarks.TYPE_SEPARATOR }); > + checkBookmarkObject(sep_3); > + > + function ensurePlace(info, parentGuid, index) { s/ensurePlace/ensurePosition/ ...let's not overload the "place" word again :) @@ +454,5 @@ > + sep_3 = yield PlacesUtils.bookmarks.fetch(sep_3.guid); > + ensurePlace(sep_3, folder_b.guid, 0); > + > + // folder_a: [sep_1], folder_b: [sep_3, sep_2] > + sep_2.index = PlacesUtils.bookmarks.DEFAULT_INDEX; maybe for the third test you could use a very large index?