355120 - Move default feed reader code to shellsvc

Status: RESOLVED FIXED

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

Description

[Asaf Romano (gone)]

Move default feed reader code to shellsvc.

something like:

readonly attribute nsILocalFile defaultFeedReader;

Comment 1

[Asaf Romano (gone)]

Attachment: patch

Comment 2

[Asaf Romano (gone)]

Attachment: patch with mac support

The mac shellsvc changes will be reviewed separately in bug 358492.

Comment 3

[:Gavin Sharp [email: [e-mail]]]

Comment on attachment 243888 [details] [diff] [review]
patch with mac support

>Index: browser/components/feeds/src/FeedWriter.js

>+      defaultReader = Cc["@mozilla.org/browser/shell-service;1"].
>+                      getService(Ci.nsIShellService).defaultFeedReader;
>     }
>+    catch(ex) { }
>+
>+    if (defaultReader && defaultReader.exists()) {

>Index: browser/components/shell/src/nsWindowsShellService.cpp

>+      rv = defaultReader->InitWithPath(path);
>+      if (NS_SUCCEEDED(rv))
>+        NS_ADDREF(*_retval = defaultReader);

What do you think about moving the exists() check to the GetDefaultReader implementation (and documenting the interface accordingly)? Getting back a non-existent nsIFile is pretty useless in this case, I think.

Comment 4

[:Gavin Sharp [email: [e-mail]]]

Comment on attachment 243888 [details] [diff] [review]
patch with mac support

moving this out of my queue...

Comment 5

[Asaf Romano (gone)]

Attachment: patch

Comment 6

[:Gavin Sharp [email: [e-mail]]]

Comment on attachment 246052 [details] [diff] [review]
patch

>Index: browser/components/shell/src/nsWindowsShellService.cpp

>+  if (path.First() == '"') {
>+    // Everything inside the quotes
>+    path = Substring(path, 1);
>+    path = Substring(path, 0, path.FindChar('"'));

This could be:
    path = Substring(path, 1, path.FindChar('"', 1) - 1);

Comment 7

[Asaf Romano (gone)]

I'm leaving the mac implementation to bug 358492 (replacing it with NS_ERROR_NOT_IMPLEMENTED for now).

Comment 8

[Asaf Romano (gone)]

Attachment: as checked in

mozilla/browser/components/feeds/src/FeedWriter.js 1.31
mozilla/browser/components/preferences/feeds.js 1.8
mozilla/browser/components/shell/public/nsIShellService.idl 1.9
mozilla/browser/components/shell/src/nsGNOMEShellService.cpp 1.20
mozilla/browser/components/shell/src/nsMacShellService.cpp 1.16
mozilla/browser/components/shell/src/nsWindowsShellService.cpp 1.41