1435166 - Don't try to move synced bookmarks into deleted parents when merging

Status: RESOLVED FIXED

(Firefox :: Sync, defect, P1)

Description

[Lina Butler [:lina]]

Here's an interesting way to reproduce this. Consider the following structure:

+ Menu
| + A
| | - B
| + C
| | - D

* Locally, add a new bookmark E into A, move D elsewhere (for example, to the toolbar), and delete the now-empty C.
* Remotely, add a new bookmark F into C, move B into the same "elsewhere" folder as above, and delete the now-empty A.

A will have a newer local timestamp, so we'll decide to keep B in A. However, A is deleted remotely, oops! Similarly, C will have a newer remote timestamp, so we'll keep D in C and undo the local move, even though C is now deleted locally.

The fix here is to always merge immediately, and never pick a parent on the other side that's deleted on our side, even if it's flagged as modified and has a newer timestamp.

Comment 1

[Lina Butler [:lina]]

Attachment: Bug 1435166 - Don't try to move synced bookmarks into deleted parents when merging.

Review commit: https://reviewboard.mozilla.org/r/217434/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/217434/

Comment 3

[Mark Hammond [:markh] [:mhammond]]

Comment on attachment 8947718 [details]
Bug 1435166 - Don't try to move synced bookmarks into deleted parents when merging.

https://reviewboard.mozilla.org/r/217434/#review223808

Looks great, thanks

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/1f7c498a92c8
Don't try to move synced bookmarks into deleted parents when merging. r=markh

Comment 5

[Narcis Beleuzu [:NarcisB]]

Backed out for xpcshell&TV failures on /test_bookmark_deletion.js.

Push: https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=7b1372468c928bcda562ef58be2c3c4d0515c13e
XPCSHELL Log: https://treeherder.mozilla.org/logviewer.html#?job_id=160552278&repo=autoland&lineNumber=2625
TV Log: https://treeherder.mozilla.org/logviewer.html#?job_id=160552226&repo=autoland&lineNumber=1991

Comment 6

[Lina Butler [:lina]]

Comment on attachment 8947718 [details]
Bug 1435166 - Don't try to move synced bookmarks into deleted parents when merging.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/217434/diff/1-2/

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/9506810cd82c
Don't try to move synced bookmarks into deleted parents when merging. r=markh

Comment 8

[Lina Butler [:lina]]

Aah, sorry about that, this needed a rebase atop bug 1435553. Fixed up and re-landed.

Comment 9

[Raul Gurzau (:RaulG)]

https://hg.mozilla.org/mozilla-central/rev/9506810cd82c