341407 - After subscribe to feed operation, rss icon is broken until web page is refreshed

Status: RESOLVED FIXED

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

Description

[Hussam Al-Tayeb]

User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.8.1a3) Gecko/20060613 Firefox/2.0a3
Build Identifier: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.8.1a3) Gecko/20060613 Firefox/2.0a3

if you set firefox to use live bookmarks and click on the rss icon in the location bar and then click on subscribe to feed and then click cancel, the rss icon will not click again

Reproducible: Always

Steps to Reproduce:
1. Firefox preferences and go to the general tab.
2. Click on 'Choose feed reader...'
3. Choose 'Use Live bookmarks' and select (enable) the option 'Open feeds in the my chosen reader automatically, skipping the preview pain'
4. Click 'OK' and close firefox preferences.
5. Go to www.archlinux.org
6. In the location bar, click on the rss icon and click on 'Subscribe to Arch Linux News Feed'.
7. In the dialog window that appears click 'cancel'
8. Try clicking on the rss icon the location bar again.

Actual Results:  
The rss icon does not click again until page is refreshed

Expected Results:  
I should be able to click on the rss icon again.

Comment 1

[Phil Ringnalda (:philor)]

Actually, no need to cancel, and more than just livemarks: it's also dead if you subscribe to one rather than cancelling, and then try to subscribe to another, and it's also dead after subscribing with a local client application.

Comment 2

[Phil Ringnalda (:philor)]

Hrm, is this just another way to get bug 257438, since we're apparently loading the feed even when we're going to skip the preview?

That seems like a good thing to avoid, for things like authentication and the fact that we'll then fail to pass on the URL to a currently unresponsive feed (data:text/html,<link rel="alternate" type="application/rss+xml" href="http://example.com:440/rss.xml"> will appear to not have done anything, until it eventually shows an error page, even if you were just trying to pass the URL to an external app without preview).

Comment 3

[Hussam Al-Tayeb]

Don't know if this helps, but after the rss icons gets broken, if you go to another tab and back, the rss icon is actually gone!

Comment 4

[Phil Ringnalda (:philor)]

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

Comment 5

[Phil Ringnalda (:philor)]

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

Comment 6

[Mike Schroepfer]

Ben - any updates on this?

Comment 7

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

Attachment: patch

While this isn't the nicest way to fix this bug, I believe this case is less common so given the timeframe it's the most preferable. There may be better long term solutions, but I think the code complexity of such solutions may pose more risk at this time.

Comment 8

[Darin Fisher]

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

You might want to rev the UUID of the interface since we've shipped this interface in FF2b1.  Also, channel.notificationCallbacks is not required to provide a nsIWebNavigation.  Though in the case of page navigations, I can't imagine why it would not.  You may want to comment about this or maybe even protect against an exception.

r=darin

Comment 9

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

Attachment: patch for checkin

Also includes patch for 336892

Comment 10

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

fixed-on-trunk

Comment 11

[Jeff Walden [:Waldo]]

You missed a caller:

http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/browser/components/feeds/src/FeedWriter.js&rev=1.3&mark=468#455

Comment 12

[Robert Sayre]

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

Comment 13

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

Attachment: fix additional caller

Comment 14

[Darin Fisher]

Comment on attachment 230627 [details] [diff] [review]
fix additional caller

r=darin

A nsIChannel is a nsIRequest, so the QI to nsIRequest should (I think) be unnecessary.

Comment 15

[Mike Connor [:mconnor]]

Comment on attachment 230627 [details] [diff] [review]
fix additional caller

approving both patches for branch landing per beltzner and myself, needed ASAP as it is blocking the fix for another blocker that's blocking the new prefwindow from landing

Comment 16

[Robert Sayre]

Checking in public/nsIFeedResultService.idl;
/cvsroot/mozilla/browser/components/feeds/public/nsIFeedResultService.idl,v  <--  nsIFeedResultService.idl
new revision: 1.1.2.4; previous revision: 1.1.2.3
done
Checking in src/FeedConverter.js;
/cvsroot/mozilla/browser/components/feeds/src/FeedConverter.js,v  <--  FeedConverter.js
new revision: 1.1.2.9; previous revision: 1.1.2.8
done
Checking in src/FeedWriter.js;
/cvsroot/mozilla/browser/components/feeds/src/FeedWriter.js,v  <--  FeedWriter.js
new revision: 1.2.2.7; previous revision: 1.2.2.6
done