Closed Bug 1040910 Opened 7 years ago Closed 6 years ago
Support XHTML in feed titles
(From Bug 436801 comment #0) > Some time ago I made an Atom 1.0 test feed to examine the differences in > rendering between various feed readers. > The feed can be found here: http://cmyk.wooptoo.com/exp/atom_vs_rss/atom.xml > and results here: http://cmyk.wooptoo.com/exp/atom_vs_rss/results > > The only bug I found in Firefox 3, was that it couldn't interpret (or > display?) XHTML elements in feed titles. Tags like <em> or <strong> are > simply ignored. > > I just wanted to know if this is an intentional behavior, or whether it is > really a bug. > For example, Google Reader renders XHTML tags in feed titles properly. This > might happen because it's a web-based reader. > > Reproducible: Always
Most of this patch are the tests - copied from Firefox.
Comment on attachment 8616793 [details] [diff] [review] Patch v1.0 Proposed fix (Didn't look at the tests) >+ * a nsIFeedTextConstruct Nit: an (395 matches for " a nsI" but 956 for " an nsI") >+ * The feed container, a nsIFeedContainer Ditto >- this._setContentText(a, entry.title); >+ let span = this._document.createElementNS(HTML_NS, "span"); >+ a.appendChild(span); >+ this._setContentText(span, entry.title); Why this change?
> (Didn't look at the tests) Me neither. I just copied them straight from Firefox. Passed: 74 Failed: 0 Todo: 0 And they all passed. >>+ * a nsIFeedTextConstruct > Nit: an (395 matches for " a nsI" but 956 for " an nsI") >>+ * The feed container, a nsIFeedContainer > Ditto Fixed. >>- this._setContentText(a, entry.title); >>+ let span = this._document.createElementNS(HTML_NS, "span"); >>+ a.appendChild(span); >>+ this._setContentText(span, entry.title); > Why this change? In Bug 436801 comment 4: > Regarding xml:base, the base is correctly used for resolving the <a> placed > on the feed title, but markup in the title (like inline image references) > will not have the correct base set. This is an essentially non-existent > problem. If you want it fixed (to 'do it right' while we are at it) it would > need some wrapper spans. I assume he knew what he was talking about.
Comment on attachment 8622385 [details] [diff] [review] Patch v2.0 fix grammar neet. Oh, I get it now, _setContentText wants to set the xml:base on the element, but we can't afford to do that for the a element because then its href might be wrong.
Attachment #8622385 - Flags: review?(neil) → review+
a=me for CLOSED TREE
You need to log in before you can comment on or make changes to this bug.