1040910 - Support XHTML in feed titles

Status: RESOLVED FIXED

(SeaMonkey :: Feed Discovery and Preview, defect)

Description

[Philip Chee]

(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

Comment 1

[Philip Chee]

Attachment: Patch v1.0 Proposed fix

Most of this patch are the tests - copied from Firefox.

Comment 2

[[e-mail]]

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?

Comment 3

[Philip Chee]

Attachment: Patch v2.0 fix grammar neet.

> (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 4

[[e-mail]]

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.

Comment 5

[Ian Neal]

a=me for CLOSED TREE

Comment 6

[Philip Chee]

http://hg.mozilla.org/comm-central/rev/cd3b2d6056ab