1432405 - Investigate using getConcreteItemGuid in PlacesController._removeRange

Status: RESOLVED FIXED

(Firefox :: Bookmarks & History, enhancement, P2)

Description

[Mark Banner (:standard8)]

In bug 1131491 I attempted to replace:

```
let tagItemId = PlacesUtils.getConcreteItemId(node.parent);
let tagGuid = await PlacesUtils.promiseItemGuid(tagItemId);
```

with

let tagGuid = PlacesUtils.getConcreteItemGuid(node.parent);

However, for some reason that didn't work as it should have done - it broke some tests.

We should investigate this & work out why.

Comment 1

[Mark Banner (:standard8)]

I did a bit more investigation, changing the code breaks at least browser/components/places/tests/browser/browser_library_delete_bookmarks_in_tags.js and possibly a few others.

On investigation, it turns out that getConcreteItemGuid doesn't handle tag queries like getConcreteItemId does. This appears to be because folders based queries are still using ids rather than guids, so getConcreteItemGuid wouldn't have anything to return (or we'd have to change it to be async, which would likely cause other issues).

Hence we need to wait for bug 824502 before we can fix this.

Comment 2

[Marco Bonardo [:mak]]

(In reply to Mark Banner (:standard8) (afk until 3rd April) from comment #1)
> I did a bit more investigation, changing the code breaks at least
> browser/components/places/tests/browser/
> browser_library_delete_bookmarks_in_tags.js and possibly a few others.
> 
> On investigation, it turns out that getConcreteItemGuid doesn't handle tag
> queries like getConcreteItemId does.

Is this still a problem?

Comment 3

[Mark Banner (:standard8)]

Post bug 1421664 this doesn't appear to be an issue, once we remove the old code, which I'm assuming bug 1293445 is likely to do.

Comment 4

[Mark Banner (:standard8)]

Bug 1293445 has removed this code.