Reeywhaar / want-my-rss

RSS features for Firefox
Other
113 stars 10 forks source link

Displaying RSS feed correctly after clicking on a RSS link in a Web page #10

Closed secou closed 5 years ago

secou commented 5 years ago

Hi,

It would be deeply helpful if the extension could display correctly a feed when ones click on a hyperlink in a web page pointing to a RSS feed, like, say, the ones we find in the New York Times RSS page !

Serge

Reeywhaar commented 5 years ago

You mean links like this? screen shot 2018-12-22 at 23 23 36

Well, the problem is there is now way to distinguish between whether link points on feed, or is it just some content link. screen shot 2018-12-22 at 23 30 40

But I think of a way: I could intercept web requests, look at their content-type and look at header, whether it conforms rss or atom spec. This way it will be even more like this was in old Firefox, because there will be no moz-extension://{ext-id}/show.html?url={feed-url} in url bar, there will be just feed url. There is a problem that many websites output feeds without proper content type, it could be text/plain or something. Also I'm afraid that appending blocking webrequest listener on every url would hit overall FF performance.

Right now there is a workaround, you can right click on a link, copy it's address and type ext+rss: in url bar and paste the link.

secou commented 5 years ago

OK I understand the problem! In Chrome, a subscribtion extension does that pretty well. https://chrome.google.com/webstore/detail/rss-subscription-extensio/nlbjncdgjeocebhnmkbbbdekmmmcbfjd But perhaps is it difficult to do the same thing now in Firefox. Thanks again for your work!

jscher2000 commented 5 years ago

Another possible workaround: add a command to the right-click context menu to load a link in the RSS viewer (show.html). As an example of how it might work (not tested):

/**** Create context menu item ****/
let linkcontext = browser.menus.create({
  id: "wantmyrss_content_link",
  title: "Open in Want my RSS",
  contexts: ["link"],
  icons: {
    "icons/icon-color.svg"
  }
});
/**** Handle context menu item click ****/
browser.menus.onClicked.addListener((menuInfo, currTab) => {
    switch (menuInfo.menuItemId) {
        case 'wantmyrss_content_link':
            // Probably only works with HTTP and HTTPS - link check TODO
            var RUrl = browser.extension.getURL('show.html') + '?url=' + encodeURIComponent(menuInfo.linkUrl);
            browser.tabs.create({
                url: RUrl,
                active: true
            }).then((newTab) => {
                /* No action */;
            }).catch((err) => {
                console.log('Error in tabs.create with "' + RUrl + '": ' + err.message);
            });
        default:
            // Error?
    }
});
Reeywhaar commented 5 years ago

@jscher2000 thank you for the concrete example! I'm still thinking about solution, especially because of @secou gave an example of extension that works.

Reeywhaar commented 5 years ago

@secou @jscher2000 I released version 0.10 which has said option, would be glad if you help me to check correctness, as I'm still in doubt of both performance, and that I don't have false positives when checking if link is feed. Cheers!

jscher2000 commented 5 years ago

The interception works for me. I tested with:

(A) The links in the right column of my blog under "Site Actions": https://www.jeffersonscher.com/ttw/

These send Content-Type: application/rss+xml; charset=UTF-8

Same Content-Type sent by: https://www.nasa.gov/rss/dyn/lg_image_of_the_day.rss

(B) The "RSS LINK" button on this podcast: https://www.npr.org/podcasts/510313/how-i-built-this

This sends Content-Type: text/xml;charset=UTF-8

So that's very promising. I don't have examples of other possible Content-Type's to check.

jscher2000 commented 5 years ago

I'm not sure how to check for false positives. These seem fine:

application/xml: https://www.startpage.com/en/opensearch.xml

text/xml: https://www.iana.org/assignments/media-types/media-types.xml

I suppose there could be XML files that contain these strings in some other context --

function isFeed(body: string): boolean {
    if (body.indexOf("<feed") !== -1) return true;
    if (body.indexOf("<rss") !== -1) return true;
    return false;
}

-- but unless you are Google or another collector of online files, computing the incidence would be difficult.

Reeywhaar commented 5 years ago

@jscher2000 Thank you for testing! Great that it works. My biggest worry is that somewhere on the internet is xml file which is not feed and contains <rss> or <feed>. Right now my logic for checking is that if request content-type contains application/rss+xml or application/atom+xml then it goes straight into viewer. If type contains text/xml or application/xml then I check for first 100+ characters and if they contain said tags. Have found no brighter way :-) I could've check for dtd's but it's also not 100% Anyway, in case of false positive, clicking on a feed link in a viewer doesn't get intercepted, so at worst you'll have to click twice, or disable interception in options. In fact, I don't even remember when I clicked on xml file which is not feed last time.

edit: wrote simultaneously :-)

jscher2000 commented 5 years ago

On the question of performance as well as false positives, since you only intercept main_frame requests, I think the risks are low.

Reeywhaar commented 5 years ago

Yeah, I was afraid at first, but it seems it doens't hurt experience much if any. My concern was that cumulative request blocking of multiple extensions would kill speed, so didn't want to add to this cauldron.

jscher2000 commented 5 years ago

I think with this change, you can reply to the review from a few days ago:

https://addons.mozilla.org/firefox/addon/want-my-rss/reviews/1257481/

Reeywhaar commented 5 years ago

Thank you for review! Replied to user.