Feed Preview Should Render Inline HTML Partly

RESOLVED INACTIVE

Status

RESOLVED INACTIVE
8 years ago
2 months ago

People

(Reporter: felix, Assigned: felix)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments)

(Assignee)

Description

8 years ago
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>).
(Assignee)

Updated

8 years ago
Assignee: nobody → ffung
(Assignee)

Updated

8 years ago
Summary: Feed Preview Should Render Inline HTML Party → Feed Preview Should Render Inline HTML Partly
(Assignee)

Comment 1

8 years ago
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)
(Assignee)

Comment 4

8 years ago
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-

Comment 6

9 months ago
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

Updated

2 months ago
Product: Firefox → Firefox Graveyard
You need to log in before you can comment on or make changes to this bug.