1067956 - Async Places Transactions: Drop functionality

Status: RESOLVED FIXED

(Firefox :: Bookmarks & History, defect)

Description

[Asaf Romano (gone)]

90% of the work was done elsewhere (see dependency graph). This is just about onDrop.

Comment 1

[Marco Mucci [:MarcoM]]

Hi Mano, can you provide a point value.

Comment 2

[Asaf Romano (gone)]

Attachment: patch

Comment 3

[Marco Bonardo [:mak]]

Comment on attachment 8491650 [details] [diff] [review]
patch

Review of attachment 8491650 [details] [diff] [review]:
-----------------------------------------------------------------

similar question to the paste bug, browser_475045.js simulates a drop and soon or later should be fixed to work also with async transactions.

::: browser/components/places/content/controller.js
@@ +1608,5 @@
>      let transactions = [];
>      let dropCount = dt.mozItemCount;
>      let movedCount = 0;
> +    let parentGuid = PlacesUIUtils.useAsyncTransactions ?
> +                     (yield insertionPoint.promiseGuid()) : null;

might we do this yield only in the useAsyncTransactions case below?
I'd not want to change onDrop behavior in the other case, and this is actually making onDrop async.

@@ +1666,5 @@
> +          transactions.push(
> +            PlacesUIUtils.getTransactionForData(unwrapped,
> +                                                 flavor,
> +                                                 parentGuid,
> +                                                 index, doCopy));

fix indentation

@@ +1671,5 @@
> +        }
> +        else {
> +          transactions.push(PlacesUIUtils.makeTransaction(unwrapped,
> +                            flavor, insertionPoint.itemId,
> +                            index, doCopy));

indent 2 more

::: toolkit/components/places/PlacesTransactions.jsm
@@ +335,5 @@
>     * @return {Promise}
> +   * @resolves either to the resolution of |execute|, in single-transaction
> +   * mode, or to the return value of the generator, in generator-mode. For an
> +   * array of transactions, there's no resolution value.
> +   * 

trailing space

Comment 4

[Asaf Romano (gone)]

https://hg.mozilla.org/integration/fx-team/rev/913568167ec5

Comment 5

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/913568167ec5