TYPE_MAYBE_FEED is defined in browser.js, but it isn't used there. That definition is only used in applications.js on Mac OS X, and applications.js then separately defines the constant for other OSes in an #ifndef XP_MACOSX. This seems unnecessarily complicated and prone to being misunderstood, and it would be better to define TYPE_MAYBE_FEED only in applications.js, where it's used, rather than in browser.js, where it isn't. Here's a patch that does that.

Comment on attachment 300450 [details] [diff] [review] patch v1: moves TYPE_MAYBE_FEED definition to applications.js r=mano

Comment on attachment 300450 [details] [diff] [review] patch v1: moves TYPE_MAYBE_FEED definition to applications.js Requesting 1.9 approval for this super-simple, very-low-risk code cleanup patch that doesn't need to go into b3.

Hmm, there's more cruft in the direct vicinity, including a case where we define kXULNS in browser.js, only use it in preferences/advanced.js in some offline storage code, and don't define it there for Windows and Linux, which presumably means the code that uses it is broken on those two platforms (although I haven't confirmed this yet; I can't figure out how to invoke that code). I could file a new bug for these additional changes, but given that they're in the same area of code and are doing the same thing (removing cruft, simplifying the code), I thought it made sense to include them in this patch. This version includes the following changes: 1. moves TYPE_MAYBE_FEED from browser.js to applications.js, as in the original patch; 2. moves kXULNS from browser.js to advanced.js; 3. removes TYPE_XUL, which isn't used anywhere; 4. removes BROWSER_ADD_BM_FEATURES, which isn't used anywhere.

Mano pointed out that we don't need kXULNS at all, actually, since we're in a XUL document, so we can just use createElement instead of createElementNS.

With campd's help I've confirmed that the list of offline storage websites is indeed broken without this patch on Windows and Linux, which is a more serious problem than I anticipated when I originally said in comment 2 that this doesn't need to be fixed for beta 3. Given that problem, this seems worth getting into that beta, so requesting blocking status and setting target milestone accordingly.

Blocking for offline storage feature ...

Comment on attachment 300693 [details] [diff] [review] patch v3: gets rid of kXULNS entirely r=mano

The fix is in: Checking in browser/base/content/browser.js; /cvsroot/mozilla/browser/base/content/browser.js,v <-- browser.js new revision: 1.957; previous revision: 1.956 done Checking in browser/components/preferences/applications.js; /cvsroot/mozilla/browser/components/preferences/applications.js,v <-- applications.js new revision: 1.43; previous revision: 1.42 done Checking in browser/components/preferences/advanced.js; /cvsroot/mozilla/browser/components/preferences/advanced.js,v <-- advanced.js new revision: 1.30; previous revision: 1.29 done