Mike Kroger Reporter
	Description • 18 years ago

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.1.1) Gecko/20061204 Firefox/2.0.0.1
Build Identifier: 2.0.0.1

I haven't tested the bug with an http:// uri, so it might only apply to chrome:// uri's.

Using a chrome:// uri for a feed handler causes Firefox to throw the following two exceptions when viewing a preview of a feed: -

Error: [Exception... "Component returned failure code: 0x80004003 (NS_ERROR_INVALID_POINTER) [nsIIOService.newChannelFromURI]" nsresult: "0x80004003 (NS_ERROR_INVALID_POINTER)" location: "JS frame :: file:///C:/Program%20Files/Mozilla%20Firefox/components/FeedWriter.js :: iconDataURIGenerator :: line 992" data: no]
Source File: file:///C:/Program%20Files/Mozilla%20Firefox/components/FeedWriter.js
Line: 992

Error: uncaught exception: [Exception... "Component returned failure code: 0x80004003 (NS_ERROR_INVALID_POINTER) [nsIIOService.newChannelFromURI]" nsresult: "0x80004003 (NS_ERROR_INVALID_POINTER)" location: "JS frame :: file:///C:/Program%20Files/Mozilla%20Firefox/components/FeedWriter.js :: iconDataURIGenerator :: line 992" data: no]

The uncaught exceptions also cause the preview page to not work correctly. Also, the Tools>Options>Feeds page behaves strangely, refusing to allow the user to select the chrome feed handler from the list.

Reproducible: Always

Steps to Reproduce:
1. There might be an easier way to reproduce the bug but I've no idea how to do that.
2. Install the Wizz RSS News Reader extension for Firefox (AMO ID - 424).
3. Add a new feed handler by adding the following lines in about:config: -

browser.contentHandlers.types.6.title - Wizz RSS News Reader
browser.contentHandlers.types.6.type - application/vnd.mozilla.maybe.feed
browser.contentHandlers.types.6.uri - chrome://wizzrss/content/addfeed.xul?url=%s

4. Try to select Wizz RSS News Reader (Subscribe to feed using) on the Tools>Options>Feeds page.
5. Try to preview a feed.
Actual Results:  
Preview page doesn't work. See screen shot here: -
http://img405.imageshack.us/my.php?image=wizzdesktopua1.jpg

Tools>Options>Feeds page doesn't work as it should.

Expected Results:  
The preview page should work normally and the Tools>Options>Feeds page should work normally.

	Mike Kroger Reporter
	Comment 1 • 18 years ago

Sorry, I forgot to mention that setting Wizz RSS News Reader as a feed handler in versions of Firefox prior to version 2.0.0.1 worked perfectly. It is only since version 2.0.0.1 that the problem has occurred.

	Adam Kowalczyk
	Comment 2 • 18 years ago

Yes, it's probably a problem with all chrome:// URIs and it shouldn't even matter if favicon is available or not. This happens because uri.prePath is useless for a chrome:// URI

717         var uri = makeURI(handlers[i].uri);
718         if (uri) 
719           new iconDataURIGenerator(uri.prePath + "/favicon.ico", menuItem)

Confirming and adjusting summary.

	Vijay
	Comment 3 • 18 years ago

I can also confirm the existence of this bug, it's only speculation, but I'm guessing it has somthing to do with the fixing of <a href=https://bugzilla.mozilla.org/show_bug.cgi?id=358878>Bug 358878</a> in 2.0.0.1, as that's the only RSS related issue I can see that's been changed since 2.0

	Asaf Romano (gone) Assignee
	Comment 4 • 18 years ago

Doing this because it would be hard for extensions to bypass the FeedWriter behavior. registerContentHandler disallows chrome:// handlers.

	Mike Kroger Reporter
	Comment 5 • 18 years ago

While we are talking about it, is it possible to change the behavior of registerContentHandler so that it will allow a chrome:// uri?

I'd like to add a "Register as Feed Handler" option to the Wizz RSS extension so that people who don't really want to hack about:config can easily resgister the extension as their feed handler with a single click.

	:Gavin Sharp [email: gavin@gavinsharp.com]
	Comment 6 • 18 years ago

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

r=me if you make these a regexp check on .scheme instead of .spec.

	Asaf Romano (gone) Assignee
	Comment 7 • 18 years ago

mozilla/browser/components/feeds/src/FeedWriter.js 1.34
mozilla/browser/components/preferences/feeds.js 1.9

	Johnny Stenback (:jst)
	Comment 8 • 18 years ago

Comment on attachment 250129 [details] [diff] [review]
as checked in

Driveby review comment:

-        if (uri) 
+        if (uri && /^https?/.test(uri.scheme))

Shouldn't that test for /^https?$/ to make sure we get it right in the case someone has some other protocol handlers laying around with a scheme that starts with http or https?

	Asaf Romano (gone) Assignee
	Comment 9 • 18 years ago

Probably, though we don't do so in other places.

	Johnny Stenback (:jst)
	Comment 10 • 18 years ago

Ok, but please file a follow up bug on cleaning that up throughout the codebase.

	Jay Patel [:jay]
	Comment 11 • 18 years ago

Comment on attachment 250129 [details] [diff] [review]
as checked in

Approved for 1.8 branch, a=jay for drivers.  Let's make sure to get that cleanup bug filed also. Thanks!

	Asaf Romano (gone) Assignee
	Comment 13 • 18 years ago

mozilla/browser/components/feeds/src/FeedWriter.js 1.2.2.28
mozilla/browser/components/preferences/feeds.js 1.1.2.8

	Asaf Romano (gone) Assignee
	Comment 14 • 18 years ago

(In reply to comment #11)
> (From update of attachment 250129 [details] [diff] [review])
> Approved for 1.8 branch, a=jay for drivers.  Let's make sure to get that
> cleanup bug filed also. Thanks!

Filed bug 366192.

	Adam Kowalczyk
	Comment 15 • 18 years ago

(In reply to comment #13)
> Created an attachment (id=250725) [details]
> branch diff for real

Err, maybe not? :)

	Mike Kroger Reporter
	Comment 17 • 18 years ago

I have tested the changes to FeedWriter.js

-        if (uri) 
+        if (uri && /^https?/.test(uri.scheme))

And it works just fine. However, the changes to feeds.js

-      row.setAttribute("image", uri.prePath + "/favicon.ico");
-      
+      if (/^https?/.test(uri.scheme))
+        row.setAttribute("image", uri.prePath + "/favicon.ico");

Still leaves me unable to select the feed handler without the favicon on the Tools>Options>Feeds page.

	Asaf Romano (gone) Assignee
	Comment 18 • 18 years ago

OK, please file a separate bug.

	Martijn Wargers (dead)
	Comment 19 • 18 years ago

Verified fixed on trunk.
I can see the bug using a 2006-12-30 build, but not anymore with a 2007-01-02 build.
Verified that it isn't a problem anymore on:
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.1.2pre) Gecko/20070207 BonEcho/2.0.0.2pre

The issue mentioned in comment 17 can still be seen in the latest builds, so I think there needs to be bug filed for that, still.