Closed Bug 380639 Opened 17 years ago Closed 6 years ago

dynamic removal of feeds/<link> elements (DOMLinkRemoved) don't cause updates to the feed discovery menu (live bookmarks)


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

Not set


(Not tracked)



(Reporter: mstreatfield, Unassigned)




(2 files)

User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv: Gecko/20070309 Firefox/
Build Identifier: Mozilla/5.0 (X11; U; Linux i686; en-US; rv: Gecko/20070309 Firefox/

this applies to the live bookmarking icon in the right hand corner of the address bar in firefox

using javascript (and AJAX) it is possible to add live bookmarks after a page has finished loading by adding <link> tags to the DOM.  in browser.js there is a handler (onLinkAdded) on the event (DOMLinkAdded) to capture this.

however, if links are removed dynamically, the live bookmarking option in the address bar does not update to show the new links - essentially there is not a onLinkRemoved handler for the DOMLinkRemoved event to capture this.

this is a missing function in mozilla/browser/base/content/browser.js

Reproducible: Always

Steps to Reproduce:
on any page run some javascript to add two links:

var headId = top.document.getElementsByTagName("head")[0];
var newLink = document.createElement('link');
newLink.rel = 'alternate';
newLink.type = 'application/rss+xml';
newLink.title = "link a";
newLink.href = "";

var newLink = document.createElement('link');
newLink.rel = 'alternate';
newLink.type = 'application/rss+xml';
newLink.title = "link b";
newLink.href = "";

and then remove a link:

Actual Results:  
live bookmarking shows "link a" and "link b"

Expected Results:  
live bookmarking to show just one link option
Component: Bookmarks → RSS Discovery and Preview
OS: Linux → All
QA Contact: bookmarks → rss.preview
Hardware: PC → All
Blocks: 396056
This is exactly what I am looking for!
For instructions on moving forward with the patch, see:
Can somebody else submit Mark's fix, or does it have to be Mark himself?
Doesn't matter, but fwiw, I don't think that patch would be accepted as is, because it introduces quite a bit of duplicated code.
More significant than the duplicated code is that it just doesn't work: Array.pop() takes off the last element of the array, it doesn't take arguments and search for them. An actual working patch would need to do that searching, would need to decide what constitutes a match if it isn't exactly "all three match" (if you add a feed with one title, change the title, then remove the link element, does it remove the feed with a matching href and type, or not?), and now that we have a single listener for DOMLinkAdded for feeds, icons, and search, would probably need to handle removing all three, and would need automated tests along the lines of the tests for adding in
Ever confirmed: true
Thanks for the info.  I've never submitted anything myself before, but it gives me some direction (and some hint of what I might be up against) if I wanted to try to do a patch myself.  
Is anyone actively working on the patch? If not I may give it a shot.
William, by all means feel free to work on it (it does not appear anyone is currently)...

Do realize that currently Firefox 3 is nearing its "Beta 4" milestone which means that any code changes are strictly looked at, so no-one can guarantee that your change will actually be in Firefox 3 (yet).  But if its done fast enough, and gets needed reviews, hopefully it can make it!
See bug 471535 comment 6 and subsequent comments for some thoughts about how to fix this.

Claudio and Matteo also mention an issue with DOMLinkRemoved in that bug, but there aren't really any details about the problem - Claudio, can you elaborate?
Summary: missing DOMLinkRemoved handler for dynamic removal of live bookmarks → dynamic removal feeds/<link> elements (DOMLinkRemoved) don't cause updates to the feed discovery menu (live bookmarks)
Summary: dynamic removal feeds/<link> elements (DOMLinkRemoved) don't cause updates to the feed discovery menu (live bookmarks) → dynamic removal of feeds/<link> elements (DOMLinkRemoved) don't cause updates to the feed discovery menu (live bookmarks)
Alternatively, we could just update the menu each time it's opened (which would also fix bug 396056).
The issue with DOMLinkRemoved is that this event is never fired. The workaround by Matteo fires a DOMLinkRemoved event when the DOMNodeRemoved event has a LINK element as original target. That is the purpose of the _FixDOMEvents function added as event handler to catch DOMNodeRemoved events right after the DOMContentLoaded event.
I would rather fix this in Gecko with DOMLinkRemoved correctly than have us incur a perf penalty with the DOMNodeRemoved listener[s] on the document.

Is there any case where DOMLinkRemoved *is* fired? I thought there was a few at least!
I saw this issue today, trying to develop a userscript that would reduce the number of feeds in pages and leave only the ones that are relative to current page.

I confirm links removed using JavaScript are still listed when opening the feeds list with the RSS icon button.
Is there anyone on this bug who would be kind enough to mentor it?
(In reply to Anthony Ricaud (:rik) (out until June 2nd) from comment #18)
> Is there anyone on this bug who would be kind enough to mentor it?

Sure, although solving this with the approach in comment 13 is trickier now with multiple processes which makes me think we should get DOMLinkRemoved working and just listen for that. I just tested and it seems to work for some removals while not others but I haven't investigated why.

To start, I would recommend making a test case showing that DOMLinkRemoved doesn't always work and attaching it in a Core dependency bug. With that bug fixed, this should be straightforward.

The relevant code has moved a bit due to E10S:
Minimal test case. The link named "Static (removed)" is removed and the link named "Dynamic (added)" is added immediately with JavaScript. Both show up in the Subscribe menu while only "Dynamic (added)" should show up.
Per policy at If this bug is not an enhancement request or a bug not present in a supported release of Firefox, then it may be reopened.
Closed: 6 years ago
Resolution: --- → INACTIVE
Product: Firefox → Firefox Graveyard
You need to log in before you can comment on or make changes to this bug.