Closed Bug 1067959 Opened 6 years ago Closed 6 years ago

Async Places Transactions: when copying an item, do not copy "special" annotations

Categories

(Toolkit :: Places, defect)

x86
macOS
defect
Not set
normal
Points:
2

Tracking

()

RESOLVED FIXED
mozilla35
Iteration:
35.2

People

(Reporter: mano, Assigned: mano)

References

Details

Attachments

(1 file)

See bug 1058566 and the original bug for the legacy implementation, bug 629620.
Points: --- → 2
Flags: qe-verify?
Flags: firefox-backlog+
Status: NEW → ASSIGNED
Iteration: --- → 35.2
Flags: qe-verify? → qe-verify-
Comment on attachment 8492229 [details] [diff] [review]
patch.diff

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

Please just verify what we do today regarding excluded annotations and aRestoring.

::: toolkit/components/places/PlacesTransactions.jsm
@@ +740,2 @@
>  // Update the documentation at the top of this module if you add or
>  // remove properties.

looks like you are not reading your own comments :)

@@ +819,5 @@
>   *        Whether or not the items are restored.  Only in restore mode, are
>   *        the guid, dateAdded and lastModified properties honored.
> + * @param [optional] aExcludingAnnotations
> + *        Array of annotations names set on any item in aBookmarksTree to
> + *        ignore. This is ignored if aRestoring is set.

I don't like the phrase, it's quite unclear:
"Array of annotations names set on any item in aBookmarksTree to ignore."

I'm not sure if it's correct to ignore it on restore, what do we do today?
Attachment #8492229 - Flags: review?(mak77) → review+
That's the current behavior actually (see PlacesRemoveItemTransaction), which makes sense of course (otherwise it's not a true "undo").
https://hg.mozilla.org/mozilla-central/rev/dca3955efe70
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla35
You need to log in before you can comment on or make changes to this bug.