Last Comment Bug 436801 - XHTML not properly supported in Feed titles
: XHTML not properly supported in Feed titles
Status: RESOLVED FIXED
:
Product: Firefox
Classification: Client Software
Component: RSS Discovery and Preview (show other bugs)
: unspecified
: x86 All
: -- normal (vote)
: Firefox 23
Assigned To: Nicholas Wilson
:
Mentors:
http://cmyk.wooptoo.com/exp/atom_vs_r...
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2008-06-01 12:18 PDT by wooptoo
Modified: 2014-07-18 12:07 PDT (History)
4 users (show)
ryanvm: in‑testsuite+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Write HTML content into the title instead of unnecessarily flattening it to plain text (1.76 KB, patch)
2010-09-01 09:18 PDT, Nicholas Wilson
sayrer: review+
Details | Diff | Review
Added some unit tests; fixed xml:base behaviour (7.06 KB, patch)
2010-09-02 09:39 PDT, Nicholas Wilson
adw: review+
Details | Diff | Review

Description wooptoo 2008-06-01 12:18:14 PDT
User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9) Gecko/2008052921 Firefox/3.0
Build Identifier: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9) Gecko/2008052921 Minefield/3.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 Tanner M. Young [:tmyoung] 2009-08-29 14:03:52 PDT
Have you tested to see if this still happens in Firefox 3.5?
Comment 2 wooptoo 2009-08-29 14:19:47 PDT
Yes, this still happens in Firefox 3.5
You can also find the Atom test feed here: http://pastebin.com/f5583531a (the site mentioned above is not always online).
Comment 3 Tanner M. Young [:tmyoung] 2009-08-29 15:06:20 PDT
CCing sayrer who will know if this is the intended behavior and if it can be fixed.
Comment 4 Nicholas Wilson 2010-09-01 09:18:59 PDT
Created attachment 471141 [details] [diff] [review]
Write HTML content into the title instead of unnecessarily flattening it to plain text

This is a simple, uncontroversial fix.

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.
Comment 5 Robert Sayre 2010-09-01 22:37:24 PDT
Comment on attachment 471141 [details] [diff] [review]
Write HTML content into the title instead of unnecessarily flattening it to plain text

r=me, but this needs a test too. xml:base can be handled another day.
Comment 6 Nicholas Wilson 2010-09-02 09:39:40 PDT
Created attachment 471530 [details] [diff] [review]
Added some unit tests; fixed xml:base behaviour

Today is another day. xml:base is easy to handle. There are also four unit tests to ensure that the base URI is added, and four to check that the title elements correctly contain the markup.
Comment 7 Drew Willcoxon :adw 2013-04-12 22:56:29 PDT
Comment on attachment 471530 [details] [diff] [review]
Added some unit tests; fixed xml:base behaviour

Review of attachment 471530 [details] [diff] [review]:
-----------------------------------------------------------------

These patches still apply cleanly.  After all this time I hate to pick nits, but... If you're not able or interested in making these changes anymore, let me know and I'll make them and land.

It's hard to read the tests due to all the very long lines.  Please add line breaks and indentation to make them more readable, like:

<title>
  <div>
    ...
  </div>
</title>

and 

is(document.getElementById("..."),
   "some very long string",
   "this should be that");

::: browser/components/feeds/src/FeedWriter.js
@@ +210,5 @@
>        "  element.removeChild(element.firstChild);" +
>        "element.appendChild(textNode);";
> +    if (text.base) {
> +      this._contentSandbox.spec = text.base.spec;
> +      codeStr += "element.setAttributeNS('"+XML_NS+"', 'base', spec);";

Please put spaces around the concatenation terms:

"element.setAttributeNS('" + XML_NS + "', 'base', spec);";
Comment 8 Drew Willcoxon :adw 2013-04-26 23:58:46 PDT
Landed: https://hg.mozilla.org/integration/mozilla-inbound/rev/e3126e00ec91

The innerHTML comparisons in the test failed because some attributes were serialized in a different order, so I made the test check the DOM instead.

Thanks for the patch!  Sorry it took so long to review and land.
Comment 9 Drew Willcoxon :adw 2013-04-27 13:57:43 PDT
Whoops, shouldn't have marked fixed yet since it's not on m-c.
Comment 10 Ryan VanderMeulen [:RyanVM] 2013-04-27 18:32:34 PDT
https://hg.mozilla.org/mozilla-central/rev/e3126e00ec91

Note You need to log in before you can comment on or make changes to this bug.