Closed Bug 959272 Opened 7 years ago Closed 7 years ago

Fix feed folder rename/move/copy and db sync issues

Categories

(MailNews Core :: Feed Reader, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
Thunderbird 29.0

People

(Reporter: alta88, Assigned: alta88)

Details

Attachments

(2 files, 2 obsolete files)

Fix this once and for all by getting rid of the feedUrl property, a terrible design. 

1. Rely only on the feeds db for folder/feed url relationship, don't get or set the feedUrl in either folder or msg databases.
2. Ensure correct old folder url and feed url are found for all cases (esp subfolder actions and cross account actions) so all the db feed urls in a folder hierarchy can be correctly updated with the new names.
Attached patch feedUrl.patch (obsolete) — Splinter Review
Assignee: nobody → alta88
Status: NEW → ASSIGNED
Attachment #8359320 - Flags: review?(mkmelin+mozilla)
Attached patch feedUrl.patch (obsolete) — Splinter Review
tweak for menupopup binding.
Attachment #8359320 - Attachment is obsolete: true
Attachment #8359320 - Flags: review?(mkmelin+mozilla)
Attachment #8359411 - Flags: review?(mkmelin+mozilla)
Comment on attachment 8359411 [details] [diff] [review]
feedUrl.patch

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

Seems ok to me. r=mkmelin

::: mailnews/extensions/newsblog/content/FeedUtils.jsm
@@ +293,5 @@
> +    this.log.debug("FeedUtils.updateSubscriptionsDS: folder changed - " + aAction);
> +    this.log.debug("FeedUtils.updateSubscriptionsDS: new folder  - " +
> +                   aFolder.filePath.path);
> +    this.log.debug("FeedUtils.updateSubscriptionsDS: orig folder - " +
> +                   aOrigFolder.filePath.path);

I think single actions should be logged in a single log statement (where you have all the necessary info included)

@@ +342,5 @@
> +                                        aNewAncestorURI, aOrigAncestorURI) {
> +    this.log.debug("updateFolderChangeInFeedsDS: aFolder       - " + aFolder.URI);
> +    this.log.debug("updateFolderChangeInFeedsDS: aOrigFolder   - " + aOrigFolder.URI);
> +    this.log.debug("updateFolderChangeInFeedsDS: aOrigAncestor - " + aOrigAncestorURI);
> +    this.log.debug("updateFolderChangeInFeedsDS: aNewAncestor  - " + aNewAncestorURI);

here too, single logging for a single thing

@@ +367,5 @@
> +    catch(ex)
> +    {
> +      this.log.error("updateFolderChangeInFeedsDS: feeds.rdf db error - " + ex);
> +      this.log.error("updateFolderChangeInFeedsDS: feeds.rdf db error for account - " +
> +                     aOrigFolder.server.prettyName);

and here

::: mailnews/local/public/nsINewsBlogFeedDownloader.idl
@@ +29,5 @@
>     * the feeds.rdf subscriptions data source.
>     */
> +  void updateSubscriptionsDS(in nsIMsgFolder aFolder,
> +                             in nsIMsgFolder aOrigFolder,
> +                             in string aAction);

please document params

::: mailnews/local/src/nsRssIncomingServer.cpp
@@ +122,5 @@
>  {
>    // Pass the selected folder on to the downloader.
>    NS_ENSURE_ARG_POINTER(aFolder);
>    nsresult rv;
> +  nsCOMPtr <nsINewsBlogFeedDownloader> rssDownloader =

drop the space after nsCOMPtr

@@ +238,5 @@
>    if (!aFolder)
>      return NS_OK;
>  
> +  nsresult rv;
> +  nsCOMPtr <nsINewsBlogFeedDownloader> rssDownloader =

nsCOMPtr<nsINewsBlogFeedDownloader>
Attachment #8359411 - Flags: review?(mkmelin+mozilla) → review+
Attached patch feedUrl.patchSplinter Review
updated.
Attachment #8359411 - Attachment is obsolete: true
Attachment #8360694 - Flags: review+
Keywords: checkin-needed
https://hg.mozilla.org/comm-central/rev/407015c80f27
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 29.0
Attached patch feedUrl2.patchSplinter Review
fix log msg/typo.
Attachment #8385041 - Flags: review+
Keywords: checkin-needed
You need to log in before you can comment on or make changes to this bug.