+++ 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.)
Not sure what the "nonempty" you added upstream since I patched bug 457621 does, but I assume it's something good, that I want?
Attachment #455389 - Flags: review?(dirkjan)
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 on attachment 455389 [details] [diff] [review] Fix v.1 So the double escaping introduced by escape|xmlescape is intentional, right?
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 <pre>", but then the XML parser would unescape that, so we escape once more, and the parser sees "No &lt;pre&gt;" which it turns into "No <pre>" which the feed reader puts into "<div class="entry">No <pre></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 on attachment 455389 [details] [diff] [review] Fix v.1 Sounds good, then.
Attachment #455389 - Flags: review?(dirkjan) → review+
Status: NEW → RESOLVED
Last Resolved: 9 years ago
Resolution: --- → FIXED
Product: mozilla.org → Release Engineering
Product: Release Engineering → Developer Services
You need to log in before you can comment on or make changes to this bug.