The default bug view has changed. See this FAQ.

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

RESOLVED FIXED

Status

Developer Services
Mercurial: hg.mozilla.org
RESOLVED FIXED
7 years ago
3 years ago

People

(Reporter: philor, Assigned: philor)

Tracking

({regression})

Details

(URL)

Attachments

(1 attachment)

(Assignee)

Description

7 years ago
+++ 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.)
(Assignee)

Comment 1

7 years ago
Created attachment 455389 [details] [diff] [review]
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?
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?
(Assignee)

Comment 4

7 years ago
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 on attachment 455389 [details] [diff] [review]
Fix v.1

Sounds good, then.
Attachment #455389 - Flags: review?(dirkjan) → review+
(Assignee)

Comment 6

7 years ago
http://hg.mozilla.org/hgcustom/hg_templates/rev/c58bb645a198
Status: NEW → RESOLVED
Last Resolved: 7 years ago
Resolution: --- → FIXED
(Assignee)

Updated

7 years ago
Depends on: 601388
Product: mozilla.org → Release Engineering

Comment 7

3 years ago
https://hg.mozilla.org/hgcustom/version-control-tools/rev/7cd4656510c3
Product: Release Engineering → Developer Services
You need to log in before you can comment on or make changes to this bug.