344279 - FeedWriter dumps error messages to the console

Status: RESOLVED FIXED

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

Description

[Adam Guthrie]

As of now, the LOG function directly dumps stuff to the console. We need to do something like Gavin did in nsSearchService. See also bug 343080 (microsummaries equivalent of this).

Comment 1

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

Ben, can you take care of this?

Comment 2

[Joey Minta]

Attachment: check feeds.log pref

Patch makes the LOG function check whether feeds.log is set to true.  If the pref is unset (as in a normal profile), or set to false, then we don't dump anything.  Those wishing to debug can set the pref to true.

Comment 3

[Ben Goodger (use ben at mozilla dot org for email)]

Comment on attachment 229844 [details] [diff] [review]
check feeds.log pref

>Index: browser/components/feeds/src/FeedWriter.js
> function LOG(str) {
>-  dump("*** " + str + "\n");
>+  var prefB = Cc["@mozilla.org/preferences-service;1"].
>+              getService(Ci.nsIPrefBranch);

Style for this file dictates:

var prefB = 
    Cc["contract-id"].
    getService(Ci...);

>+  var shouldLog = false;
>+  try {
>+    shouldLog = prefB.getBoolPref("feeds.log");
>+  } catch (ex) {}

and

try { 
 //..
}
catch (e) {
}

r=[e-mail] with those nits picked.

Comment 4

[Ben Goodger (use ben at mozilla dot org for email)]

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

Comment 5

[Joey Minta]

Landed on trunk.
Checking in browser/components/feeds/src/FeedWriter.js;
/cvsroot/mozilla/browser/components/feeds/src/FeedWriter.js,v  <--  FeedWriter.js
new revision: 1.4; previous revision: 1.3

Comment 6

[pal-moz]

cause this ?
http://forums.mozillazine.org/viewtopic.php?p=2383959#2383959

Comment 7

[Joey Minta]

(In reply to comment #6)
> cause this ?
> http://forums.mozillazine.org/viewtopic.php?p=2383959#2383959
> 
I *highly* doubt it.  If someone has reason to believe this checkin caused this regression, I'd like to hear it, but please don't blindly speculate.  This checkin didn't touch anything in the actual FeedWriter object.

Comment 8

[Joey Minta]

Comment on attachment 229844 [details] [diff] [review]
check feeds.log pref

Requesting 1.8.1 approval.  This is a minimal change, using code that has been tested in a variety of components.  It fixes a significant bug in a new feature.  Testcase is to view any feedpage, for instance the one linked from planet.mozilla.org.

Comment 9

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

Comment on attachment 229844 [details] [diff] [review]
check feeds.log pref

a=drivers. Please go ahead and land this on the MOZILLA_1_8_BRANCH.

Comment 10

[Joey Minta]

Landed on branch
Checking in browser/components/feeds/src/FeedWriter.js;
/cvsroot/mozilla/browser/components/feeds/src/FeedWriter.js,v  <--  FeedWriter.js
new revision: 1.2.2.6; previous revision: 1.2.2.5
done