[Tracker][Add-On API] process_refs gets wrong directory entry_ref

[ahwayakchih]

Hi,

I just tried nightly (after a looong time) inside virtualbox to try to rebuild my tiny apps on Haiku and noticed different behaviour than the one described in BeBook (​https://www.haiku-os.org/legacy-docs/bebook/TheTracker_AddOnProtocol.html#process_refs).

I may be doing something wrong, but it looks like the Tracker passes wrong entry_ref to the add-on application. Instead of passing entry_ref to the directory, i'm getting entry_ref to the selected file.

I managed to workaround that by using BEntry.IsFile() and getting parent directory before continuing, but that was not needed before.

Was that intentionally changed without changing header files (first parameter to process_refs is called directory in header files)?

Regards, ahwayakchih

[ahwayakchih]

Test Add-On

[ahwayakchih]

Test shows that Tracker passes entry_ref to one of the selected entries, instead of the directory they are located in.

[ahwayakchih]

I think the bug is in here:

​https://github.com/haiku/haiku/blob/master/src/kits/tracker/ContainerWindow.cpp#L3228

For easier reading, section of code (lines 3226-3232):

    const entry_ref* modelRef = TargetModel()->IsContainer()
            && selectionList->ItemAt(0) != NULL
        ? selectionList->ItemAt(0)->TargetModel()->EntryRef()
        : TargetModel()->EntryRef();

    LaunchInNewThread("Add-on", B_NORMAL_PRIORITY, &AddOnThread, refs,
        addonRef, *modelRef);

On line 3228, it gets entry_ref of the first item, instead of entry_ref of first item's directory.

I may be wrong, but i think that whole ternary condition is incorrect. shouldn't it be more like:

    const entry_ref* modelRef = !TargetModel()->IsContainer()
            && selectionList->ItemAt(0) != NULL
        ? selectionList->ItemAt(0)->TargetModel()->EntryRef()
        : TargetModel()->EntryRef();

    LaunchInNewThread("Add-on", B_NORMAL_PRIORITY, &AddOnThread, refs,
        addonRef, *modelRef);

(notice the first condition - added missing "!" there :). That would be much better (although line 3228 should still be changed to get entry_ref to parent directory), but still not ideal: don't know if that may happen in practice, but if TargetModel was not container and there would be no item selected, we would still get entry_ref to that weird TargetModel, in which case it probably would be better to ignore whole action and return earlier (and/or show some error message?).

[ahwayakchih]

After looking into it some more, now i know why the condition was written like that: ​https://github.com/haiku/haiku/commit/2634c39731fdbbbd5324944bffb5075abe71619f

So, if we do not want to pass the query / virtual directory as ref, only first of their items, then we should get first item's directory.

What other TargetModel there can be? If it is not a directory, virtual directory nor a query?

Maybe it should just check if it is directory (in which case, get TargetModel entry ref) or not (in which case get entry_ref of first items parent directory)?

Something like this:

    const entry_ref* modelRef;
    if (TargetModel()->IsDirectory())
    {
        modelRef = TargetModel()->EntryRef();
    }
    else if (selectionList->ItemAt(0) != NULL)
    {
        // This is just a pseudo code for example purposes.
        // There is no ParentEntryRef function AFAIK :).
        modelRef = selectionList->ItemAt(0)->TargetModel()->ParentEntryRef();
    }
    else
    {
        // Not a directory, and no items selected?
        // Show some error
        // Cleanup
        delete refs; // refs is created above.
                     // we could move condition that to prevent object creation instead
        return;
    }

    LaunchInNewThread("Add-on", B_NORMAL_PRIORITY, &AddOnThread, refs,
        addonRef, *modelRef);

[korli]

It should be a regression resulting from this ​commit

[axeld]

Looks like OpenTargetFolder was using the directoryRef incorrectly, and jscipione then fixed it in the wrong way. This might also explain why archiving using the Tracker add-on suddenly didn't work anymore.

[waddlesplash]

Bumping to 'blocker' priority, as this breaks nearly all Tracker addons.

[jscipione]

Fixed in hrev49314