Closed Bug 1293458 Opened 5 years ago Closed 5 months ago

Extend `Bookmarks.fetch` to allow a `parentGuid` without an `index`


(Toolkit :: Places, defect, P3)




81 Branch
Tracking Status
firefox81 --- fixed


(Reporter: lina, Assigned: mkaply)



(1 file, 1 obsolete file)

Specifying a parent without an index would fetch all of a parent's children, sorted by the index. (Suggestion from bug bug 1274108, comment 59).
Priority: -- → P3

Shouldn't this just be removing this line:

All of the queries in the file already take parentGuid into account when doing their queries.

(In reply to Mike Kaply [:mkaply] from comment #1)

Shouldn't this just be removing this line:

Yes, with a test checking that the default value works (likely it needs a "defaultValue: this.DEFAULT_INDEX," instead of "requiredIf")

Assignee: nobody → mozilla

Posted before your comment. I'll make the additional changes.

also, I'm not sure it's just going to work because it seems comment 0 is requesting a behavior change, so fetching without an index fetches all the children... currently with parent and index it only fetches one... As I said I can't check atm due to lack of time left (sorry).

we have await fetchBookmarkByPosition(fetchInfo, options); this seems to require the addition of a fetchBookmarksByParent similar to fetchBookmarksByURL

Yeah, this is way more complicated then it looks. It looks like you can't combine various fetch items at all. So if you specify a GUIDPrefix, you can't use Tags for instance.

So my use case - specifying a GUID prefix and a parentGUID wouldn't even work.

But Lina's use case is pretty straightforward, so I can still do a patch for that.

Attachment #9168871 - Attachment description: Bug 1293458 - Don't require index when fetching with parentGuid. r?mak! → Bug 1293458 - Allow for fetching children of a parentGuid. r?Standard8!
Attachment #9168871 - Attachment is obsolete: true
Pushed by
Add the ability to fetch bookmarks by parent GUID. r=Standard8
Closed: 5 months ago
Resolution: --- → FIXED
Target Milestone: --- → 81 Branch
You need to log in before you can comment on or make changes to this bug.