The default bug view has changed. See this FAQ.

XHTML not properly supported in Feed titles

RESOLVED FIXED in Firefox 23

Status

()

Firefox
RSS Discovery and Preview
RESOLVED FIXED
9 years ago
3 months ago

People

(Reporter: wooptoo, Assigned: Nicholas Wilson)

Tracking

unspecified
Firefox 23
x86
All
Points:
---
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

(URL)

Attachments

(2 attachments)

(Reporter)

Description

9 years ago
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
Have you tested to see if this still happens in Firefox 3.5?
(Reporter)

Comment 2

8 years ago
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).
CCing sayrer who will know if this is the intended behavior and if it can be fixed.
Status: UNCONFIRMED → NEW
Ever confirmed: true
(Assignee)

Comment 4

7 years ago
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.
Attachment #471141 - Flags: review?(sayrer)

Comment 5

7 years ago
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.
Attachment #471141 - Flags: review?(sayrer) → review+
(Assignee)

Comment 6

7 years ago
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.
Attachment #471530 - Flags: review?(sayrer)
Attachment #471530 - Flags: review?(sayrer) → review?(gavin.sharp)

Comment 7

4 years ago
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);";
Attachment #471530 - Flags: review?(gavin.sharp) → review+

Comment 8

4 years ago
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.
Status: NEW → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED

Comment 9

4 years ago
Whoops, shouldn't have marked fixed yet since it's not on m-c.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---

Updated

4 years ago
Status: REOPENED → ASSIGNED
https://hg.mozilla.org/mozilla-central/rev/e3126e00ec91
Assignee: nobody → ncw33
Status: ASSIGNED → RESOLVED
Last Resolved: 4 years ago4 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → Firefox 23

Updated

3 years ago
See Also: → bug 1040910
See Also: → bug 1321247
You need to log in before you can comment on or make changes to this bug.