348227 - Live Bookmarks should take metadata from the Feed, not the page

Status: RESOLVED FIXED

(Firefox Graveyard :: RSS Discovery and Preview, defect)

Description

[Robert Sayre]

If you click on the link below with prefs set to automatically open live bookmarks, you'll get a live bookmark with the title of this page.

http://www.xml.com/2002/12/18/examples/rss20.xml.txt

We have the feed data around when we add the bookmark, so we should add some optional trailing arguments to addLivemark.

Comment 1

[Robert Sayre]

Oh, this is worse than I first thought. I think we should block on this--it's easy to fix though.

Comment 2

[Mike Beltzner [:beltzner, not reading bugmail]]

Robert, we'll take a fix, but not block the release on this issue.

Comment 3

[Robert Sayre]

Attachment: Add optional parameters to the browser livemark functions

Comment 4

[Asaf Romano (gone)]

Comment on attachment 235449 [details] [diff] [review]
Add optional parameters to the browser livemark functions

>Index: browser/base/content/browser-places.js
>===================================================================

>-  addLiveBookmark: function PCH_addLiveBookmark(url) {
>+  addLiveBookmark: function PCH_addLiveBookmark(url, feedTitle, feedSubtitle) {
>     var ios = 
>         Cc["@mozilla.org/network/io-service;1"].
>         getService(Ci.nsIIOService);
>     var feedURI = ios.newURI(url, null, null);
>     
>     var browser = gBrowser.selectedBrowser;
>     
>     // TODO: implement description annotation
>-    //var description = this._getDescriptionFromDocument(doc);
>-    var title = browser.contentDocument.title;
>+    //var description;
>+    //if (arguments.length > 2)
>+    //  description = feedSubtitle;
>+    //else
>+    //  description = BookmarksUtils.getDescriptionFromDocument(doc);

#if 0 this instead.


>Index: browser/components/feeds/public/nsIFeedResultService.idl
>===================================================================


bump the uuid.

> 
>   /**
>    * Adds a URI to the user's specified external feed handler, or live 
>    * bookmarks. 
>    * @param   request
>    *          The request for the feed document

remove the documentation for the removed param.


>-  void addToClientReader(in nsIRequest request, in AString uri);
>+  void addToClientReader(in AString uri, in AString title, in AString subtitle);

s/AString uri/AUTF8String uri.


>Index: browser/components/feeds/src/FeedConverter.js
>===================================================================

>               break;
> 
>             default:
>               LOG("unexpected handler: " + handler);
>               // fall through -- let feed service handle error
>             case "bookmarks":
>             case "client":
>               try {
>-                feedService.addToClientReader(this._request, result.uri.spec);
>+                var feed = result.doc;
>+                feed.QueryInterface(Ci.nsIFeed);

make this |var feed = result.doc.QueryInterface(Ci.nsIFeed);|

Comment 5

[Robert Sayre]

Attachment: fix nits

Comment 6

[Asaf Romano (gone)]

Comment on attachment 235463 [details] [diff] [review]
fix nits

r=mano

Comment 7

[Robert Sayre]

Checking in browser/base/content/browser-places.js;
/cvsroot/mozilla/browser/base/content/browser-places.js,v  <--  browser-places.js
new revision: 1.8; previous revision: 1.7
done
Checking in browser/base/content/browser.js;
/cvsroot/mozilla/browser/base/content/browser.js,v  <--  browser.js
new revision: 1.697; previous revision: 1.696
done
Checking in browser/components/feeds/public/nsIFeedResultService.idl;
/cvsroot/mozilla/browser/components/feeds/public/nsIFeedResultService.idl,v  <--  nsIFeedResultService.idl
new revision: 1.7; previous revision: 1.6
done
Checking in browser/components/feeds/src/FeedConverter.js;
/cvsroot/mozilla/browser/components/feeds/src/FeedConverter.js,v  <--  FeedConverter.js
new revision: 1.19; previous revision: 1.18
done
Checking in browser/components/feeds/src/FeedWriter.js;
/cvsroot/mozilla/browser/components/feeds/src/FeedWriter.js,v  <--  FeedWriter.js
new revision: 1.12; previous revision: 1.11
done

Comment 8

[Mike Beltzner [:beltzner, not reading bugmail]]

Comment on attachment 235463 [details] [diff] [review]
fix nits

a=beltzner on behalf of 181drivers

Comment 9

[Phil Ringnalda (:philor)]

*** Bug 351077 has been marked as a duplicate of this bug. ***

Comment 10

[Phil Ringnalda (:philor)]

*** Bug 251447 has been marked as a duplicate of this bug. ***