1312494 - Mobile Bookmarks left pane query points at wrong bookmark id

Status: RESOLVED FIXED

(Firefox :: Sync, defect, P1)

Description

[Thom Chiovoloni [:tcsc] (ex-moco)]

The result of this is having no items under "Mobile Bookmarks" in the places organizer.

I have "place:folder=335" as the query url.  My mobile bookmarks root is at 742, and it seems like my old mobile bookmarks "root" (before bug 1302901 landed) was at 335.

(Assigning to kit per request in irc)

Comment 1

[Lina Butler [:lina]]

Attachment: Bug 1312494 - Fix up queries that point to the wrong folder in `_ensureMobileQuery`.

`_ensureMobileQuery` now checks and fixes mobile query URLs. This patch also
removes incorrect and unused query migration code.

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

Comment 2

[Lina Butler [:lina]]

Comment on attachment 8803978 [details]
Bug 1312494 - Fix up queries that point to the wrong folder in `_ensureMobileQuery`.

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

Comment 3

[Thom Chiovoloni [:tcsc] (ex-moco)]

Comment on attachment 8803978 [details]
Bug 1312494 - Fix up queries that point to the wrong folder in `_ensureMobileQuery`.

https://reviewboard.mozilla.org/r/88166/#review87112

::: toolkit/components/places/Database.cpp
(Diff revision 2)
>      // Delete the old folder.
>      rv = DeleteBookmarkItem(folderIds[i]);
>      if (NS_FAILED(rv)) return rv;
>    }
>  
> -  // Delete any left pane queries that point to the old folder. We'll

From discussion in IRC, now that we leave these around, I'd feel a little better about this if you made it so the validator reported an error if we end up syncing these items up to the server for some reason.

Simply adding `ignored = true` for them in createClientRecordsFromTree should be enough to make them show up as `serverUnexpected`.

Comment 4

[Lina Butler [:lina]]

Comment on attachment 8803978 [details]
Bug 1312494 - Fix up queries that point to the wrong folder in `_ensureMobileQuery`.

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

Comment 5

[Thom Chiovoloni [:tcsc] (ex-moco)]

Comment on attachment 8803978 [details]
Bug 1312494 - Fix up queries that point to the wrong folder in `_ensureMobileQuery`.

https://reviewboard.mozilla.org/r/88166/#review87118

Looks good to me.

Comment 6

[Lina Butler [:lina]]

Comment on attachment 8803978 [details]
Bug 1312494 - Fix up queries that point to the wrong folder in `_ensureMobileQuery`.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/88166/diff/3-4/

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/11758049a838
Fix up queries that point to the wrong folder in `_ensureMobileQuery`. r=tcsc

Comment 8

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/11758049a838