Closed Bug 492884 Opened 12 years ago Closed 11 years ago

getMostRecentFolderForFeedURI should use the livemarks service instead of annos directly

Categories

(Toolkit :: Places, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla1.9.2a1

People

(Reporter: dietrich, Assigned: mak)

Details

(Whiteboard: [TSnappiness])

Attachments

(1 file, 4 obsolete files)

using the livemark service would allow using the cache instead of hitting the db directly.
Whiteboard: Tsnap
Assignee: nobody → mak77
Attached patch patch v1.0 (obsolete) — Splinter Review
Attachment #384100 - Flags: review?(dietrich)
Flags: in-testsuite?
Comment on attachment 384100 [details] [diff] [review]
patch v1.0

>+  /**
>+   * Determines whether the feed URI is a currently registered livemark.
>+   *
>+   * @param aFeedURI
>+   *        Feed URI to look for.
>+   *
>+   * @returns the found livemark folder id, or -1 if nothing was found.
>+   */
>+  long long getLivemarkForFeedURI(in nsIURI aFeedURI);
> 

can you kill the whitespace on the following line

also, should be getLivemarkIdForFeedURI to be more explicit about what's returned.

>   /**
>-   * TODO: this should use the livemark service's cache of folder ids (bug 492884).
>+   * Get the most recently folder item id for a feed URI.

s/ly//

>+
>+const LMANNO_FEEDURI = "livemark/feedURI";

we should probably hang the internal annos off of PlacesUtils. please file a bug for this.
Attachment #384100 - Flags: review?(dietrich) → review+
(In reply to comment #2)
> >+
> >+const LMANNO_FEEDURI = "livemark/feedURI";
> 
> we should probably hang the internal annos off of PlacesUtils. please file a
> bug for this.

filed bug 499743
Attached patch patch v1.1 (obsolete) — Splinter Review
addressed comments
Attachment #384100 - Attachment is obsolete: true
Attached patch patch v1.2 (obsolete) — Splinter Review
wrong one
Attachment #384456 - Attachment is obsolete: true
Attached patch patch v1.3 (obsolete) — Splinter Review
ugh, lost the test.
Attachment #384457 - Attachment is obsolete: true
Attached patch patch v1.4Splinter Review
fix a typo, this should be finally ready for check-in.
Attachment #384465 - Attachment is obsolete: true
http://hg.mozilla.org/mozilla-central/rev/be7dbb414de8
Status: NEW → RESOLVED
Closed: 11 years ago
Flags: in-testsuite? → in-testsuite+
Resolution: --- → FIXED
Whiteboard: Tsnap → [TSnappiness]
Target Milestone: --- → mozilla1.9.2a1
You need to log in before you can comment on or make changes to this bug.