477714 - Enhancement Request: [RSS]Provide UI for subscription to feeds (RSS, atom, etc.)

Status: RESOLVED DUPLICATE of bug 852828

(Firefox for Android Graveyard :: General, enhancement)

Description

[Madhava Enros [:madhava]]

The equivalent UI in desktop firefox actually serves two purposes -- first, it indicateds that one or multiple feeds are available, and, second, it provides a mechanism by which the user can subscribe to the feed of his or her choice.  Both of these are represented by the RSS icon in the URL bar.

In fennec, we can provide both through use of an extended site identity button (parallel with search plugins in bug 477620).  See this mockup:

http://people.mozilla.com/~madhava/files/mobile/2008-11-18/site_button.png

Comment 1

[Matti Väisänen]

I agree with Madhava.

However, I have couple comments improvement ideas to the proposed design.

> http://people.mozilla.com/~madhava/files/mobile/2008-11-18/site_button.png

In the linked image above, a menu item takes full width of the screen and it consists of high-level information on the topic (i.e. "This site has a feed") and a button (for the actual action). The image does not explain what would happen if the site/page would contain multiple feeds (e.g. https://wiki.mozilla.org/Main_Page) but I could assume that the informative text would be "This site has feeds" and the button would invoke another list of the feeds available for subscription. Would it work like this?

The menu would be able to display more options (available for instant activation without scrolling the menu first) if the commands would be listed in two columns after/below the area for Larry. So, I would suggest 2-column layout for wide screen / landscape orientation and  1-column layout for narrow screen / portrait orientation.

Menu could list immediately all feeds available from (head part part of) HTML doc so that the user would not have to open yet another list for seing what is available for her. If the menu would have 2-column layout, it would hesitate greater number of options than 1-columns layout.

Typically web pages provide names for feeds available from the head part of the HTML docs. The menu item for a feed subscription could show the Title of the feed so that the user would know to what particular feed s/he is subscribing to instead of whatever feed (provided by the site): e.g. MozillaWiki RSS feed or MozillaWiki Atom feed.

For usability and simplicity reasons, a menu item should stand for a single action only and it should be functional as a whole. So, I would remove the button and make the whole menu item as an interactive object. If it is seen important that the menu item informs its functionality clearly for the user, the clarification could be shown below the Feed title.

For more information, see a suggestion for extending the site menu of Fennec:
https://wiki.mozilla.org/Mobile/UI/Designs/TouchScreen/Fennec_1.1%2B/Extended_site_menu

Comment 2

[Madhava Enros [:madhava]]

Based on the changes in 1.1, I think that we should look at having a single item in the site menu when there are any feeds offered by a site that would be called something like "Subscribe to feed(s)."  If there was only one available, we could name the feed in small text under the main menu item title.  If there were multiple feeds, the small text could just say "n feeds available."

Tapping on the "Subscribe to feeds" item would have to bring up some secondary UI to allow the user to choose which feed(s) he or she wanted to subscribe to.

Comment 3

[Matti Väisänen]

Attachment: Feed subscription UI in Fennec Site menu (with components available in version 1.1)

Comment 4

[Matti Väisänen]

Based on the changes in 1.1, the Feed subscription UI of the Site menu should look and behave as detailed in the PDF attached to comment #3.

Some notes related to the designs above:

• Probably the Site menu button should provide the user with some kind of visual hint that there are feeds available in the Site menu. Adding an extra icon (even a small-scale) might make the Site menu button to look crowded, so could we use e.g. indicative Background color there (if it was not reserved for other purposes)? Or colored border?

• Probably the buttons for subscribing feeds and adding plug-ins should have (add-on and feed) icons to improve their discoverability. Basically, these options differ slightly from the other Site menu options because they are about components/content provided by the site -- Not just functions provided by the browser.

I am not sure if I used the correct Chrome-provided component for Choice list in the PDF (attached to comment #3). I copied it from the Tools/Settings UI of Fennec (v. 1.1) because it looks the same as in Madhava´s sketch for list of feeds: http://www.flickr.com/photos/madhava_work/4352154856/  

If it was the correct one, (this can be a bit off-topic and should be reported in the correct bug/s in Bugzilla) I can see some issues come up with the Choice list of Fennec in context Subscribing to a feed: 

• Choice list does not contain Cancel option. You should be able to Cancel the subscription operation, if you didn´t find any desired feed in the list.

• Does the Choice list have to contain the "Done" button, if it is for making a single selection only? Wouldn´t it be more direct that you could select the desired item simply just by tapping it without any further (extra) confirmation with the Done button? Basically, you will need the "Done" option in the case of multi-selection only.

• Does the Choice list have to contain always the check marks? Obviously, they are needed in the Multi-selection list. In case of feed subscription, you are selecting a single item only. Could the Single-selection choice list indicate the currently selected option only when it really matters (e.g. in the Settings)?

• Should the Choice list contain (an optional) Title bar? In case of feed list, it could have the title "Subscribe to feed".

Comment 5

[Matti Väisänen]

Attachment: Feed subscription UI in Fennec Site menu (with PROPOSAL for new CHOICE LIST)

See the image for a proposal for solving the problems with the Choice list (see Comment #4):

1. A single selection list -> Tapping an item selects it and passes it on and closes the list.

2. Cancel by tapping the close button (of the list) or just outside of the list.

3. Choice list has a title.

Comment 7

[jukka.miettinen]

Attachment: WIP patch

WIP patch, just to show where am I going roughly at the moment.

Comment 8

[jukka.miettinen]

Attachment: Updated WIP patch, previous one was broken

Comment 9

[jukka.miettinen]

Attachment: Updated WIP patch

Feeds components isn't working well yet, and you shouldn't use remote.tabs true with this version. 

Now I'll start looking into the changes that are needed in order to get this working with e10s.

Comment 10

[jukka.miettinen]

Attachment: Updated WIP patch

Works now with remote.tabs=true and dialogs are now working. 

Known issues:
* Default preferences missing
* Feed preview broken 
* After feed detection claims that subsequents web pages have also feeds available

Comment 11

[jukka.miettinen]

Attachment: Feeds detection + UI working in e10s (except for feed preview)

Comment 12

[jukka.miettinen]

Attachment: Updated WIP patch

yummy references almost gone, feed preview still broken

Comment 13

[jukka.miettinen]

Attachment: Updated WIP patch

Updates: 
* Feeds work now with an external feed reader
* about:feeds working too with an external reader

Should fix:
* Feed preview acts in a weird way, Fennec is giving Save/Close options instead of showing the i.e rss page
* Live bookmarks references are still visible here and there
* License blocks are not up-to-date or they are missing 

What to do with:
* Bloglines / My Yahoo / Google readers? (I know how to get those working...)

Comment 14

[jukka.miettinen]

Attachment: Updated WIP patch

Now Web handlers, i.e My Yahoo and friends, are working for about:feeds. 

Preview doesn't work still, but I think I have some idea how to get that working.

Comment 15

[Mark Finkle (:mfinkle) (use needinfo?)]

* You have nsIYummyAsyncPlacesVisitAdder in this patch, but we do not want to include that. It's part of Yummy that should not be in Fennec.
* We really need to make sure that we are not adding any new strings, not already in Firefox, if possible. If we need more/new strings, we should have a good reason.
* Same for images. Let's stick with all images that are already used in Firefox. We can style the page in a separate bug.
* locales/en-US/chrome/feeds.properties - this should not be needed. remove.
* Any livemark strings and code should not be used. We are not using any livemark functionality
* nsIMIMEInfo.idl should not be in this patch. It is part of the toolkit, so we already have access to it.
* All JS component files should be based on the Firefox version, unless changes are needed. If changes are needed, please list the changes somewhere in this bug so we are aware of them.
* Any ported C++ components should be cleared of any non-mozilla terms and features.
* FeedConverter.js has lines in it that look like it has been preprocessed (the mozilla build system does this). It makes me want to use the real source file, not the one in the patch.

More comments on feeds.js are coming...

Comment 16

[jukka.miettinen]

Thanks for the feedback!

(In reply to comment #15)
> * You have nsIYummyAsyncPlacesVisitAdder in this patch, but we do not want to
> include that. It's part of Yummy that should not be in Fennec.

Yes, I will remove this. 

> * We really need to make sure that we are not adding any new strings, not
> already in Firefox, if possible. If we need more/new strings, we should have a
> good reason.

I will try to find needed strings from Firefox side. 

> * Same for images. Let's stick with all images that are already used in
> Firefox. We can style the page in a separate bug.

Yep.

> * locales/en-US/chrome/feeds.properties - this should not be needed. remove.

Ok.

> * Any livemark strings and code should not be used. We are not using any
> livemark functionality

I will do this next. Much more nicer to review code that is actually needed.

> * nsIMIMEInfo.idl should not be in this patch. It is part of the toolkit, so we
> already have access to it.

Ok, I will check this. 

> * All JS component files should be based on the Firefox version, unless changes
> are needed. If changes are needed, please list the changes somewhere in this
> bug so we are aware of them.

Sorry, I'm not following you here. Are we talking about license blocks or what?

> * Any ported C++ components should be cleared of any non-mozilla terms and
> features.

Ok, I will try to find these.

> * FeedConverter.js has lines in it that look like it has been preprocessed (the
> mozilla build system does this). It makes me want to use the real source file,
> not the one in the patch.

Hmm, I didn't know that these kind of things can happen. How to undo the damage easily? Or how to prevent this from happening again?

> More comments on feeds.js are coming...

Comment 17

[jukka.miettinen]

Attachment: Updated WIP patch

Mark, I didn't make yet modifications based on your comments. This version has now support for My Yahoo and friends Web handlers and you can change the default reader from the settings. 

I have a bit silly approach in that because I had to hard code RSS mime type to get the external feed reader handler even though I don't of course have any real content from whom to ask when I dynamically generate the list of possible feed readers for the settings.

Comment 18

[Mark Finkle (:mfinkle) (use needinfo?)]

(In reply to comment #16)
> Thanks for the feedback!

> > * All JS component files should be based on the Firefox version, unless changes
> > are needed. If changes are needed, please list the changes somewhere in this
> > bug so we are aware of them.
> 
> Sorry, I'm not following you here. Are we talking about license blocks or what?

Nope. I mean, make sure you are using the latest version of these files from the mozilla-central/browser/components/feeds

Also, do this for any other files, like the .idl files, that we need to copy from Firefox.

> > * FeedConverter.js has lines in it that look like it has been preprocessed (the
> > mozilla build system does this). It makes me want to use the real source file,
> > not the one in the patch.
> 
> Hmm, I didn't know that these kind of things can happen. How to undo the damage
> easily? Or how to prevent this from happening again?

Lines like:

//@line 44 "/builds/slave/linux_build/build/toolkit/content/debug.js"
and
//@line 37 "/builds/slave/linux_build/build/browser/components/feeds/src/GenericFactory.js"


This is because the file was preprocessed. If you use the latest source from mozilla-central, you see that we don't even do this anymore :)

There have been many changes to the files since add-ons like Yummy originally ported them. Use the latest source from mozilla-central, not other ports, and you'll be fine.

Comment 19

[jukka.miettinen]

Attachment: Update WIP patch based on Mark's feedback

Thank you for the feedback.

Improvements:
* Idl files are now identical with Firefox' ones
* My changes are now merged with Firefox' feed component source files (FeedConverter.js & FeedWriter.js)
* nsIYummyAsyncPlacesVisitAdder.idl & nsIYummyAsyncPlacesVisitAdder.js removed

Still to do:
* Couldn't get rid off nsIMIMEInfo.idl for some reason, compilation went fine but on run-time WebContentConverter stopped working. Have to look into to that later. 
* Preview page doesn't work still
* Cleaning up feeds data on WebProgress:LocationChange is causing a lot of problems, sometimes window IDs don't match for some reason and I tried some hacky approaches to get feeds data cleaned up when IDs get mixed up but no luck yet. I think there might be a bug on event handling on xulrunner side but haven't been able to prove that yet.

Comment 20

[jukka.miettinen]

Attachment: patch version 1

Still preview doesn't work but I think now it could be a good time to start reviewing. I know that I haven't cleaned up all the live bookmark related stuff but the functionality is really close to done.

Comment 21

[jukka.miettinen]

To be done:
- I have to check can I use existing localized strings
- I have to remove nsIMIMEInfo.idl
- I have remove all livemark strings and code

If you want I can provide diffs against original feeds files from Firefox to go through FeedConverter.js & FeedWriter.js line by line as you proposed.

Comment 22

[jukka.miettinen]

Attachment: patch version 2

I wasn't able to use any existing localized strings. For example, in Firefox, we have strings such as "Subscribe to this feed using" or "Always use %S to subscribe to feeds" but I don't see them fitting the current UI design. So we're missing "Subscribe to feed", "Skip preview", "Feeds" and maybe "Default feed reader". I'm not sure about the last one, wasn't able to find a document describing what to use together with the list of possible feed readers in Settings view.  

I also removed all livemark strings and code that I was able to find. There are some parts of the code that I would like to remove but I'm not happy to do so before I know for sure that they don't mess with the feed preview page. And of course there are some listeners that I don't see useful but because they are not documented, can't say for sure are they useless.  

Still to do:
- remove nsIMIMEInfo.idl
- what's wrong with feeds preview page?

Comment 23

[Naoki Hirata :nhirata (please use needinfo instead of cc)]

Testers note: please regress bug 606313 when regressing this bug.

Comment 24

[u367160]

Attachment: feeds patch

review page is working and many other problems fixed now

Comment 25

[u367160]

Attachment: feeds patch

fixed remaining issues and code cleaned up

Comment 26

[u367160]

Attachment: my work

this is the changes I made based on the old patch, I created this patch for make the reviewing easier.
basically feeds broken because of e10s, old FeedWriter in chrome process does not have access to preview pages dom content anymore, so I moved it to as a framescript and cleaned it up. moved some functionality needs chrome access to the feeds.js.
fixed other miner issues with same cause like feeds number accumulating in all tabs and appears mess up by changing original "feeds.feeds" array to two dimensional array.
popup menulist in preview page now synchronised with the one in settings page.
site menu button also fixed by removed the surrounding boxes.
also changes which make feeds working.

Comment 27

[Doug Turner (:dougt)]

will not block.

Comment 28

[u367160]

Attachment: new improved feeds patch

improvements from the last version of the patch
1. removed custom added preference "feeds.skipPreview", "feeds.defaultMethod"
and use mozilla default preferences for feeds.
2. all feed options including external feed applications, web feed handlers and
preview option combined into one combo-box in settings
3. preview page feed handler selection now remembers user's last selection.
4. code optimized and cleaned.
5. feeds settings now apply to the use cases not only when user click
"subscribe to" button in view menu but also when user open a feed URL in anyway
in browser.
btw, this is my last patch probably

Comment 29

[Oleg Romashin (:romaxa)]

Comment on attachment 475466 [details] [diff] [review]
patch version 2

this patch is outdated... new patch attached

Comment 30

[u367160]

Attachment: feeds patch 1.0

updated patch so it apply and works with latest source tree

Comment 31

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Mark, the attached patch is waiting for review during the last 7 months now. Has it been dropped from your list and you still wanna do it or should we find someone else to review it?

Comment 32

[Mark Finkle (:mfinkle) (use needinfo?)]

Comment on attachment 503761 [details] [diff] [review]
feeds patch 1.0

We don't have plans to take a built-in RSS reader anymore.

Comment 33

[Henrik Skupin [:whimboo][⌚️UTC+1]]

So what about forwarding a subscription to an external service like Google Reader? I think that's what most of the users usually do.

Comment 35

[u367160]

Sorry I can't take this enhancement request, I worked on this because I wanted to contribute Firefox on MeeGo. It has been quiet long time and I have too many other things going on now. Someone else please take this from here.

Comment 36

[Kevin Brosnan [Ex-Mozilla]]

This bug has already been reset to unowned.

Comment 37

[Peter Retzer (:pretzer)]

Bug 852828 added the ability to subscribe to feed.