Async Places Transactions: Drop functionality

RESOLVED FIXED in Firefox 35

Status

()

RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: mano, Assigned: mano)

Tracking

unspecified
Firefox 35
x86
Mac OS X
Points:
3
Dependency tree / graph
Bug Flags:
firefox-backlog +
in-testsuite +
qe-verify -

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

90% of the work was done elsewhere (see dependency graph). This is just about onDrop.
Assignee: nobody → mano
Hi Mano, can you provide a point value.
Status: NEW → ASSIGNED
Iteration: --- → 35.2
Flags: qe-verify?
Flags: needinfo?(mano)
Flags: firefox-backlog+
Points: --- → 3
Flags: needinfo?(mano)

Updated

4 years ago
Flags: qe-verify? → qe-verify-
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
Attachment #8491650 - Flags: review?(mak77) → review+
https://hg.mozilla.org/mozilla-central/rev/913568167ec5
Status: ASSIGNED → RESOLVED
Last Resolved: 4 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → Firefox 35
You need to log in before you can comment on or make changes to this bug.