Created attachment 562872 [details] RSS Atom Feed Showing Bug In RSS Feeds, we will strip all tags when the type of any field happens to be "html" or "xhtml", as in the example RSS Atom attachment. Note that while the 'type' attribute may appear in any feed, only the Atom spec defines it. Sometimes, stripping tags from entries will result in the message change the meaning of the message. As a result, we should try to (conservatively) render some HTML, such as (<b>, <i>, or <s>).
Summary: Feed Preview Should Render Inline HTML Party → Feed Preview Should Render Inline HTML Partly
Created attachment 562934 [details] [diff] [review] Feed Preview Should Render Some HTML in Titles Summary of Changes: - Created new interface function createSimpleDocumentFragment - Run titles through createDocumentFragment which parses the HTML safely into a DOM representation - Reparse the DOM representation stripping out everything outside a whitelist This code might (probably) need some refactoring. Hoping review could help with that.
Attachment #562934 - Flags: review?(philringnalda)
Component: Bookmarks & History → RSS Discovery and Preview
QA Contact: bookmarks → rss.preview
As far as I remember, feed preview used to be a security nightmare. I don't think we want to (re)open this can of worms unless there's a very compelling reason.
Comment on attachment 562934 [details] [diff] [review] Feed Preview Should Render Some HTML in Titles According to https://wiki.mozilla.org/Modules/Firefox#Feed_Preview I'd guess you're looking for Mano as a reviewer.
Attachment #562934 - Flags: review?(philringnalda) → review?(mano)
This patch doesn't parse anymore HTML than it already does. The current feed preview already calls createDocumentFragment once on the unfiltered body of an RSS story. We're just calling it on the title now. I'm not really in any position to debate the past security concerns with FeedPreview but I just wanted to make of note of it.
Comment on attachment 562934 [details] [diff] [review] Feed Preview Should Render Some HTML in Titles It seems to me that the right criteria would be "all inline elements allowed". The way this is written now, you're encouraging the use of deprecated tags (b/i/u/strong) rather than the recommended way of styling (in short, span). Also note that any element you're accepting could come with some attributes. That could be problematic for the class attribute. All that said, the FeedProcessor is going to be rewritten sometime soon (or not so soon). Thus I'm not so sure it's worth spending time on minor issues like this.
Attachment #562934 - Flags: review?(mano) → review-
Per policy at https://wiki.mozilla.org/Bug_Triage/Projects/Bug_Handling/Bug_Husbandry#Inactive_Bugs. If this bug is not an enhancement request or a bug not present in a supported release of Firefox, then it may be reopened.
Status: NEW → RESOLVED
Last Resolved: 9 months ago
Resolution: --- → INACTIVE
You need to log in before you can comment on or make changes to this bug.