The default bug view has changed. See this FAQ.

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

NEW
Unassigned

Status

()

Firefox
RSS Discovery and Preview
10 years ago
4 months ago

People

(Reporter: Mark Streatfield, Unassigned)

Tracking

(Blocks: 1 bug)

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments)

(Reporter)

Description

10 years ago
User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.8.1.3) Gecko/20070309 Firefox/2.0.0.3
Build Identifier: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.8.1.3) Gecko/20070309 Firefox/2.0.0.3

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 = "http://linka.com/feed.xml";
headId.appendChild(newLink);

var newLink = document.createElement('link');
newLink.rel = 'alternate';
newLink.type = 'application/rss+xml';
newLink.title = "link b";
newLink.href = "http://linkb.com/feed.xml";
headId.appendChild(newLink);

and then remove a link:

headId.removeChild(headId.firstChild);
Actual Results:  
live bookmarking shows "link a" and "link b"

Expected Results:  
live bookmarking to show just one link option
(Reporter)

Comment 1

10 years ago
Created attachment 264751 [details] [diff] [review]
a potential patch for the issue described in this ticket
Component: Bookmarks → RSS Discovery and Preview
OS: Linux → All
QA Contact: bookmarks → rss.preview
Hardware: PC → All

Updated

10 years ago
Blocks: 396056

Comment 2

10 years ago
This is exactly what I am looking for!

Comment 3

10 years ago
For instructions on moving forward with the patch, see: http://developer.mozilla.org/en/docs/Getting_your_patch_in_the_tree

Comment 4

10 years ago
Can somebody else submit Mark's fix, or does it have to be Mark himself?

Comment 5

10 years ago
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 http://mxr.mozilla.org/seamonkey/source/browser/base/content/test/
Status: UNCONFIRMED → NEW
Ever confirmed: true

Comment 7

10 years ago
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.  

Comment 8

9 years ago
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!
Duplicate of this bug: 451738
Duplicate of this bug: 471535
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).

Comment 14

8 years ago
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!
Duplicate of this bug: 478552

Comment 17

3 years ago
I saw this issue today, trying to develop a userscript that would reduce the number of feeds in http://500px.com/ 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:
* https://mxr.mozilla.org/mozilla-central/search?string=Link%3AAddFeed
* https://mxr.mozilla.org/mozilla-central/source/browser/base/content/browser-feeds.js

Comment 20

2 years ago
Created attachment 8596180 [details]
dynamic-feed-removal-test-case.html

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.
You need to log in before you can comment on or make changes to this bug.