Closed Bug 477714 Opened 13 years ago Closed 8 years ago

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

Categories

(Firefox for Android Graveyard :: General, enhancement)

ARM
Android
enhancement
Not set
normal

Tracking

(fennec-)

RESOLVED DUPLICATE of bug 852828
Tracking Status
fennec - ---

People

(Reporter: madhava, Unassigned)

References

Details

Attachments

(2 files, 17 obsolete files)

1.11 MB, application/pdf
Details
379.87 KB, image/png
Details
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
tracking-fennec: --- → ?
Summary: Provide UI for subscription to feeds → Provide UI for subscription to feeds (RSS, atom, etc.)
Blocks: 477628
tracking-fennec: ? → 1.0-
Assignee: nobody → madhava
Target Milestone: --- → Future
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
Depends on: 561352
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.
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".
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.
tracking-fennec: 1.0- → ?
Duplicate of this bug: 491400
tracking-fennec: ? → 2.0+
Attached patch WIP patch (obsolete) — Splinter Review
WIP patch, just to show where am I going roughly at the moment.
Attachment #464475 - Attachment is obsolete: true
Attached patch Updated WIP patch (obsolete) — Splinter Review
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.
Attachment #464542 - Attachment is obsolete: true
Attached patch Updated WIP patch (obsolete) — Splinter Review
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
Attachment #464596 - Attachment is obsolete: true
Attachment #465488 - Attachment is obsolete: true
Attached patch Updated WIP patch (obsolete) — Splinter Review
yummy references almost gone, feed preview still broken
Attachment #466464 - Attachment is obsolete: true
Attached patch Updated WIP patch (obsolete) — Splinter Review
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...)
Attachment #467230 - Attachment is obsolete: true
Attachment #468399 - Flags: feedback?(mark.finkle)
Attached patch Updated WIP patch (obsolete) — Splinter Review
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.
Attachment #468399 - Attachment is obsolete: true
Attachment #469114 - Flags: feedback?(mark.finkle)
Attachment #468399 - Flags: feedback?(mark.finkle)
* 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...
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...
Attached patch Updated WIP patch (obsolete) — Splinter Review
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.
Attachment #469114 - Attachment is obsolete: true
Attachment #469683 - Flags: feedback?(mark.finkle)
Attachment #469114 - Flags: feedback?(mark.finkle)
(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.
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.
Attachment #469683 - Attachment is obsolete: true
Attachment #470095 - Flags: feedback?(mark.finkle)
Attachment #469683 - Flags: feedback?(mark.finkle)
Attached patch patch version 1 (obsolete) — Splinter Review
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.
Attachment #470095 - Attachment is obsolete: true
Attachment #473014 - Flags: review?(mark.finkle)
Attachment #470095 - Flags: feedback?(mark.finkle)
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.
Attached patch patch version 2 (obsolete) — Splinter Review
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?
Attachment #473014 - Attachment is obsolete: true
Attachment #475466 - Flags: review?(mark.finkle)
Attachment #473014 - Flags: review?(mark.finkle)
Summary: Provide UI for subscription to feeds (RSS, atom, etc.) → [RSS]Provide UI for subscription to feeds (RSS, atom, etc.)
Testers note: please regress bug 606313 when regressing this bug.
tracking-fennec: 2.0+ → 2.0b3+
Attached patch feeds patch (obsolete) — Splinter Review
review page is working and many other problems fixed now
Attachment #487440 - Flags: review?(mark.finkle)
Attached patch feeds patch (obsolete) — Splinter Review
fixed remaining issues and code cleaned up
Attachment #487440 - Attachment is obsolete: true
Attachment #487569 - Flags: review?(mark.finkle)
Attachment #487440 - Flags: review?(mark.finkle)
Attached patch my work (obsolete) — Splinter Review
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.
Attachment #487576 - Flags: review?(mark.finkle)
will not block.
tracking-fennec: 2.0b3+ → 2.0-
Attached patch new improved feeds patch (obsolete) — Splinter Review
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
Attachment #487569 - Attachment is obsolete: true
Attachment #487576 - Attachment is obsolete: true
Attachment #500784 - Flags: review?(mark.finkle)
Attachment #487569 - Flags: review?(mark.finkle)
Attachment #487576 - Flags: review?(mark.finkle)
Comment on attachment 475466 [details] [diff] [review]
patch version 2

this patch is outdated... new patch attached
Attachment #475466 - Attachment is obsolete: true
Attachment #475466 - Flags: review?(mark.finkle)
Attached patch feeds patch 1.0 (obsolete) — Splinter Review
updated patch so it apply and works with latest source tree
Attachment #500784 - Attachment is obsolete: true
Attachment #503761 - Flags: review?(mark.finkle)
Attachment #500784 - Flags: review?(mark.finkle)
tracking-fennec: 2.0- → ---
Whiteboard: [fennec-4.1?]
tracking-fennec: --- → -
Whiteboard: [fennec-4.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?
Assignee: madhava → ekrem.tomur
Status: NEW → ASSIGNED
Comment on attachment 503761 [details] [diff] [review]
feeds patch 1.0

We don't have plans to take a built-in RSS reader anymore.
Attachment #503761 - Flags: review?(mark.finkle)
So what about forwarding a subscription to an external service like Google Reader? I think that's what most of the users usually do.
Duplicate of this bug: 800844
Assignee: ekrem.tomur → nobody
Severity: normal → enhancement
OS: All → Android
Product: Fennec → Firefox for Android
Hardware: All → ARM
Summary: [RSS]Provide UI for subscription to feeds (RSS, atom, etc.) → Enhancement Request: [RSS]Provide UI for subscription to feeds (RSS, atom, etc.)
Target Milestone: Future → ---
Attachment #503761 - Attachment is obsolete: true
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.
This bug has already been reset to unowned.
Status: ASSIGNED → NEW
Bug 852828 added the ability to subscribe to feed.
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → DUPLICATE
Duplicate of bug: 852828
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in before you can comment on or make changes to this bug.