576205 - changelog Atom feed is hard to read with long lines wrapped in <pre>

Status: RESOLVED FIXED

(Developer Services :: Mercurial: hg.mozilla.org, defect)

Description

[Phil Ringnalda (:philor)]

+++ This bug was initially created as a clone of Bug #457621 +++

The changelog Atom feed takes commit messages, both single-line ones of arbitrary length from -m and multi-line ones (either multiline in the sense of hard returns at some random character, or multiline in the sense of two newlines for new paragraphs), and stuffs them into a <content type="xhtml"><pre>...</pre></content> wrapper.

Because we rarely use either sort of multi-line messages, but we very frequently use single-line ones that are wider than my feed reader's window, reading the changelog feed involves constant horizontal scrolling.

There's no simple solution while retaining the ideological purity of a type="xhtml" content element (that would require a much fancier nl2<br/>&<p></p> instead of the simple nl2br/addbreaks filter), but ideological purity doesn't soothe my scrolling finger, and there's very, very little purity in "my inline XHTML is plain text stuffed in a <pre>."

(We fixed this in August 2008, but lost the fix in the hg upgrade.)

Comment 1

[Phil Ringnalda (:philor)]

Attachment: Fix v.1

Not sure what the "nonempty" you added upstream since I patched bug 457621 does, but I assume it's something good, that I want?

Comment 2

[Dirkjan Ochtman (:djc)]

nonempty replace empty cset messages with "(none)" -- it's probably good, but I'd hope we don't actually need it here. Nonetheless, should be harmless at worst.

Comment 3

[Dirkjan Ochtman (:djc)]

Comment on attachment 455389 [details] [diff] [review]
Fix v.1

So the double escaping introduced by escape|xmlescape is intentional, right?

Comment 4

[Phil Ringnalda (:philor)]

Yeah, the double escaping in type="html" is where "ideological purity of a type="xhtml" content element" becomes pure by comparison.

Start with a plaintext commit of "No <pre>" - <content type="html"> says "the string that your XML parser gives you will be HTML source" so we escape once to "No &lt;pre&gt;", but then the XML parser would unescape that, so we escape once more, and the parser sees "No &amp;lt;pre&amp;gt;" which it turns into "No &lt;pre&gt;" which the feed reader puts into "<div class="entry">No &lt;pre&gt;</div>" so the person reading the feed sees "No <pre>" again, rather than what single escaping would do, seeing "No" (and the whole rest of the page in a <pre>, in older feed readers, though modern ones will just strip an unclosed HTML tag, or more rarely decide that you meant to escape it).

Comment 5

[Dirkjan Ochtman (:djc)]

Comment on attachment 455389 [details] [diff] [review]
Fix v.1

Sounds good, then.

Comment 6

[Phil Ringnalda (:philor)]

http://hg.mozilla.org/hgcustom/hg_templates/rev/c58bb645a198

Comment 7

[Gregory Szorc [:gps]]

https://hg.mozilla.org/hgcustom/version-control-tools/rev/7cd4656510c3