Atom <title type="html"> should have HTML entities and numeric character references replaced

RESOLVED FIXED in Thunderbird 5.0b1

Status

defect
RESOLVED FIXED
11 years ago
4 years ago

People

(Reporter: philor, Assigned: alta88)

Tracking

Trunk
Thunderbird 5.0b1
Dependency tree / graph
Bug Flags:
wanted-thunderbird3 +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments, 2 obsolete attachments)

Although we correctly don't resolve double-escaped entities and NCRs in RSS titles (bug 277243), we incorrectly don't resolve them in Atom titles with type="html" - FeedParser.xmlUnescape() resolves &gt;, &lt;, and &amp;, rather than everything from &rsquo; to &#8217;

The ideal way of fixing it would be to switch entirely to using the toolkit feed parsing service; a reasonable alternative would be to use its nsIScriptableUnescapeHTML service.
Flags: wanted-thunderbird3+
Depends on: 450543
Component: RSS → Feed Reader
Product: Thunderbird → MailNews Core
Duplicate of this bug: 557461
Posted patch tired of the bad subjects.. (obsolete) — Splinter Review
Attachment #524078 - Flags: review?(bugzilla)
Assignee: nobody → alta88
Status: NEW → ASSIGNED
Comment on attachment 524078 [details] [diff] [review]
tired of the bad subjects..

Sorry for the delay in getting to this, do you have an example feed I could verify this on?

I think also we should be considering getting unit tests (probably via mozmill, but maybe xpcshell if feeds support it), for this, so that we can help to ensure stability for feeds in future. I'm willing to help you form some basic tests if you need it.

>diff --git a/mailnews/extensions/newsblog/content/FeedItem.js b/mailnews/extensions/newsblog/content/FeedItem.js

>+    var unescapeHTML = Components.classes["@mozilla.org/feed-unescapehtml;1"].
>+                       getService(Components.interfaces.nsIScriptableUnescapeHTML);
>+    title = unescapeHTML.unescape(title);

Nit: might as well just make this:

Components.classes["@mozilla.org/feed-unescapehtml;1"]
          .getService(Components.interfaces.nsIScriptableUnescapeHTML)
          .unescape(title);

Note that we tend to prefer having the dot at the start of the line for mailnews items.
Posted patch fix nit. (obsolete) — Splinter Review
a sample feed might be:
http://www.nakedcapitalism.com/feed/atom

if there is an existing test for feeds you can point me to, i can adapt it for this small change.  i think it would be a bit of a curve though if this change were to start new tests for feeds, as i have no familiarity with them.
Attachment #524078 - Attachment is obsolete: true
Attachment #524078 - Flags: review?(mbanner)
Attachment #528403 - Flags: review?(mbanner)
Attachment #528403 - Attachment is patch: true
Attachment #528403 - Attachment mime type: text/x-patch → text/plain
Posted patch fix nit v2Splinter Review
Ok, I told you wrong. It should have been title = ...
Attachment #528403 - Attachment is obsolete: true
Attachment #529653 - Flags: review+
Attachment #528403 - Flags: review?(mbanner)
Posted patch Skeleton TestSplinter Review
I did start writing a test, but got stuck with how to select account central. I'm putting this here so that hopefully someone with a bit more time can pick it up, I'll post on the newsgroup about it.

However, I'm happy that the main patch can land without this.
Flags: in-testsuite?
Checked in: http://hg.mozilla.org/comm-central/rev/bdd02cca51f5
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 3.3a4
Flags: in-testsuite?
You need to log in before you can comment on or make changes to this bug.