1154457 - Extra lines for nested MarkDown lists

Status: RESOLVED FIXED

(Bugzilla :: Creating/Changing Bugs, defect)

Description

[Albert Ting]

Attachment: v1

The change in bug 1059684 causes an unwanted effect for nested lists. A comment such as below:

- foo
  - bar
    - baz

produces:

- foo
                              <--- one blank line
  - bar
                              <--- two blank lines

    -baz

The specific change in the other ticket was:

   Bugzilla/Markdown.pm:
   -    $text =~ s/ {2,}\n/ <br$self->{empty_element_suffix}\n/g;
   +    $text =~ s/\n/<br$self->{empty_element_suffix}\n/g;
 
Reverting this patch fixes the nested list problem, yet breaks the original <pre> issue. 

Attached is one way I solved it.

Comment 1

[:glob ✱]

Comment on attachment 8592453 [details] [diff] [review]
v1

i really wanted to review this, but it's increasingly unlikely i'll have time.  opening up the review for others.

Comment 2

[Albert Ting]

Attachment: v2

Updated and simplified version.  The prior version would caused fenced code blocks to be skipped

Comment 3

[Frédéric Buclin]

Comment on attachment 8679589 [details] [diff] [review]
v2

>+    # strip trailing newlines created by DoLists
>+    $text =~ s/\n</</g;

Despite this fixes the problem, this code will catch all occurences of \n<, not only those generated by _DoLists(). This can probably have other side-effects, right? I.e. remove newlines where it shouldn't. IMO, the right fix is for lists to not insert <br> between items. This doesn't make sense.

Comment 4

[Frédéric Buclin]

(In reply to Frédéric Buclin from comment #3)
> IMO, the right fix is for lists to not insert <br> between items. This doesn't make sense.

Ah, I see where <br> comes from. _RunSpanGamut() converts \n into <br>.

In that case, I don't see why the change mentioned in comment 0 cannot be reverted. Do you have a testcase where reverting this change would trigger some problem?

Comment 5

[Frédéric Buclin]

dkl: could you explain why the change mentioned in comment 0 has been done? And would it regress something to revert this specific change?

Comment 6

[David Lawrence [:dkl]]

Comment on attachment 8679589 [details] [diff] [review]
v2

Review of attachment 8679589 [details] [diff] [review]:
-----------------------------------------------------------------

r=dkl

Comment 7

[David Lawrence [:dkl]]

To ssh://[e-mail]/bugzilla/bugzilla.git
   4bd1de9..ae22da8  master -> master