Non-leading whitespace mangled in bug description and comments

Bug 2625's description consists of one sentence. Why would it have leading whitespace anyway?

Bug2625's first comment deminstrates the issue (the affects, private and subscribe line should be indented). The description on this bug (Bug2627) demonstrates the issue affects descriptions too.

< Whitespace! >
 -------------
      \ _
       \ (_)
        \ ^__^ / \
         \ (oo)\_____/_\ \
            (__)\ ) /
                ||----w ((
                || ||>>

Don't have a cow, man. Bunnies are much cuter.

(\_/)
(O.o)
(> <)

Reported bug 2720 about the affects etc. lines.

So what is left to deem this bug fixed, or is it a meta-bug (with no Launchpad support for them)?

Well, neither of our ascii arts are displaying correctly on production (mine due to non-proportional font and whitespace munching, mpts due to non-proportional fond). So the issue is still open.

With the current text to html implementation, the following issues are still aparent:
    * whitespace runs inside a line is not preserved. Leading whitespace is preserved.
    * a proportional font is used to display comments.

While there is still some heuristics in the algorithm, it seems to do a pretty good job for most cases, including most code samples.

If you want to see what the the comments would be like with a monospace font, try the following bookmarklet:

javascript:(function(){var x=document.styleSheets[1];x.insertRule('.boardCommentBody{font-family:monospace}',x.cssRules.length)})()

Perhaps a reasonable heuristic is to treat indented text as text that is to have whitespace preserved and displayed in a non-proportional font.

Unless we use a rule like this, I think we have to use a non-proportional font for all bug description/comment text in order to fix this bug.

At any rate, any problems with leading whitespace appear to have been resolved, which leaves (a) runs of whitespace inside lines and (b) proportional fonts preventing ASCII art from working.

It's unfortunate that certain people, such as me, use spaces to format itemized lists in text. Other than that, I think that leading whitespace should, indeed, suggest a fixed-width font (as the python faq wizard does, for instance). I guess I'd just give up using the leading spaces once I'd learned that Malone would ruin my comments with them!

E-mail interface crud is not a use case (bug 2720). Cows and bunnies are not a use case. ASCII art mockups of dialogs etc could be a use case, but I'm inclined to think they should be attachments anyway.

We can't reasonably display anything as preformatted text as long as it's being displayed in such a narrow column. We can, however, solve the problem of people *expecting* whitespace won't get munched by saying textarea {font-family: caption}.

The use cases would be:

    Fred enters some text in a bug comment form. He submits the form and
    sees his comment exactly as he entered it.

    Fred replies to a Malone bug mail. The next day he visits the bug page
    and sees his comments exactly as he entered them.

ASCII art is a good demonstration and test - we cannot predict what users are going to enter. We can only predict that users will be pissed if they spend 20 mins creating an ASCII art diagram and Malone does not display it correctly. We can also predict that if we can't render the cow and bunny correctly we can't render other data that people might consider more legitimate correctly, such as stack traces, tracebacks, arbitrary terminal cut & paste.

If we cannot meet these generic use cases until we move to CSS3, then perhaps we can work around the issue. Two options I can think of are:

    - Sniff browser, and use proprietary CSS or HTML hacks to display
      things correctly.

    - Provide a link in the bug comment subject and next to the bug
      description that lets you view the raw text.

Does the fix for this bug will also apply to Rosetta and other parts of launchpad or is this something only related to bug comments and descriptions?

Bug 61123 describes the same problem in translation strings and I assume the same problem happens in tickets comments and descriptions.

This does not apply to translations.

By default, the whitespace would be there.

Somewhere, there's probably an 's/ */ /g' transformation.

This is probably fixable by finding and locating that *extra code that was added* and blasting it away so normality is restored.

Just to add my comment on this. The removal of whitespace lines inside text is indeed problematic. Specifically, I want to copy and paste a short diff describing a change. What I would like to see is: (exchanging ' ' for '_' because of the munger)

__baseline text
+__if foo:
+____do bar
+__do baz

What I see instead is:

__baseline text
+if foo:
+do bar
+do baz

Especially for copying diffs of python code, this is especially problematic, as there is no way to tell whether the 'do baz' clause is part of the if case or not.

I ran into this on bug #215522. Has there been any progress on this?

I realize that I can post a patch as an attachment, where it will not get munged. However, for a quick 5-line diff discussion, it is a lot of overhead to create the diff file, upload it, etc. Not to mention it doesn't show up directly in the comments, so it is more difficult to *talk* about the diff.

I second John's appeal, and would note that now Launchpad has its nice 2-column layout with less portlets, it's quite feasible to have an 80-column-wide text display, even on a laptop.

Wrapping them around <pre> would be one solution. I'm not sure why it hasn't been done....

The reason I didn't do it was because I didn't know about {white-space: pre-wrap} and its browser-specific siblings. But even if I had known about them and tried them, I would soon have discovered that (1) there was no equivalent value in then-current versions of Safari and (2) pre-wrap causes an ugly hanging indent in Konqueror 3.5.

I'll check what the situation is with modern browsers.

I'm suffering of this

<pre>
df -m /dev | egrep '/dev$' | sed 's/ */ /g'| cut -d ' ' -f 2
</pre>

turns on:
df -m /dev | egrep '/dev$' | sed 's/ */ /g'| cut -d ' ' -f 2

but there is two spaces between / and * on 's/ */ /g'

look at:
https://bugs.launchpad.net/bugs/317161

This is important to fix, but I will not have time to get to it.

We're tracking this sort of bug in launchpad-web now.

I've read over the bug and all its comments at least twice now. What exactly is the problem? How can I reproduce it?

No matter, I think I've got it.

There's some suggestions for approaches to handle this problem in bug #76604 too (which was a duplicate).

I'm downgrading this from high: while a certain set of things don't work, few users are inconvenienced and we have many things much more pressing to do.

> I'm downgrading this from high: while a certain set of things don't
> work, few users are inconvenienced and we have many things
> much more pressing to do.

Given the bug number (#2627!), clearly. :-)

Another use case -- unreadable disassembly paste, even after inserting the leading whitespaces.

  https://bugs.launchpad.net/sbcl/+bug/720382/comments/2

Stupid question: Why not just use <pre> in the presence of leading whitespace?

On Thu, Apr 7, 2011 at 2:02 PM, Nikodemus Siivola <
<email address hidden>> wrote:

> Stupid question: Why not just use <pre> in the presence of leading
> whitespace?
>

In my case, even with <pre> whitespace is mangled

--
Iuri Diniz
http://iuridiniz.com [Sou um agitador, não um advogado]
http://blog.igdium.com [Linux on Limbo]

I meant that why can't launchpad convert

 things
 like this

into

<pre>
 things
 like this
</pre>

instead of inserting non-breaking spaces like it now does?

The majority of the issues described in this bug, the most important issues, are addressed. The remaining issues are low priority.

Okay, now this is off the LP team's RADAR and the contribution agreement situation has changed, I'd like to have a stab at fixing it by using <pre> replacement instead of &nbsp; conversion.

From memory, this was using a generic conversion function from an external library, rather than anything specific within Launchpad. Is that still the case. Has anyone got any pointers as to functions/files that I should be looking at to help narrow things down?

  "So instead we wrote our own text wrapper, MailWrapper."
  http://bazaar.launchpad.net/~launchpad-pqm/launchpad/devel/view/head:/lib/canonical/launchpad/doc/textformatting.txt

  http://bazaar.launchpad.net/~launchpad-pqm/launchpad/devel/view/head:/lib/lp/bugs/browser/bug.py ::
  BugTextView::comment_text().build_message(text)

covers "View for simple text page displaying information for a bug.", but that class appears to be returning something in its render() that is text/plain, rather than the full '+bug' view.

Paul, afaik,

MailWrapper as the name suggests is only relevant to mail.

BugTextView is a quirky pre-api machine-oriented view and also not relevant.

You should start from lib/lp/bugs/templates/bugtask-index.pt which shows

        <tal:description
            define="global description context/bug/description/fmt:obfuscate-email/fmt:text-to-html" />

so it's coming from the bug.description field and then having two filters applied, of which the important one is text-to-html, which is defined in stringformatter.py.

I suspect just using a <pre> will have unacceptable knock-on effects such as page widening. But, by all means play with it and see if you can get something better.

I think to actually deploy this it ought to be behind a feature flag <https://dev.launchpad.net/FeatureFlags> so that it can be gradually rolled out across different browsers.

... also, while fixing this narrow issue is worthwhile, we should also
think about just how user text in Launchpad really ought to be
presented. Is it really always monospace? Is there going to be
markup (markdown?)? When does it wrap?

Martin: thanks for those two leads. Could I have comments on the linked branch, which (tries) to introduce a 'bugs.text_to_html.compress_whitespace' and presume that is default. This is written blind, and without testing.

In addition to adding the flag description, the documentation at:

  https://dev.launchpad.net/FeatureFlags

states that one must add a new scope class in:

  lib/lp/services/features/scopes.py

but this file appears to be mostly empty. Is all that is needed to query a FeatureFlag the single 'getFeatureFlag()' call?

Paul, thanks so much for tackling this, ascii art connoisseurs
everywhere will rejoice when it's fixed.

I think to make sure if this is actually what you want you are going
to need to run it. It's not trivial to get lp running but it's not
all that bad, and people in #launchpad-dev can help.

As far as the actual patch
<http://bazaar.launchpad.net/~sladen/launchpad/compress-whitespace-lp2627/revision/14130>

 * you are turning this on everywhere that uses text_to_html, not just
bug descriptions, and that's probably too much

 * there is still the issue I raised above that pre blocks never(?)
wrap. (I guess you can change that in css but you would have to do
that.)

 * is doing <pre> one line at a time a good idea?

The addition of the feature flag check is basically ok though:

 * the name implies it's only for bugs but as I said this will change
just about all variable text in lp

 * you need to document the flag in flags.py

> In addition to adding the flag description, the documentation at:
> https://dev.launchpad.net/FeatureFlags
>states that one must add a new scope class in:
> lib/lp/services/features/scopes.py
> but this file appears to be mostly empty.

 * it's not empty
http://bazaar.launchpad.net/~sladen/launchpad/compress-whitespace-lp2627/view/head:/lib/lp/services/features/scopes.py

 * you only need to do this if you want to add a new scope, which is
not necessary here.

> Is all that is needed to query a FeatureFlag the single 'getFeatureFlag()' call?

for checking an existing flag yes, but here also
https://dev.launchpad.net/FeatureFlags#Adding_and_documenting_a_new_feature_flag

there are also probably some tests to update, but that can wait until
you're happy with the way it looks.

cheers