WordPress.org

Making WordPress.org

Opened 7 years ago

Closed 9 months ago

#20 closed defect (fixed)

Possible for user's to break layout when adding forum posts

Reported by: siobhyb Owned by: SergeyBiryukov
Milestone: Priority: normal
Component: Support Forums Keywords: has-patch needs-testing
Cc:

Description

When a user wraps text in their forum posts with "<li>" tags - The layout of the page breaks.

For an example of this see - http://wordpress.org/support/topic/please-ignore-just-testing-something?replies=1#post-4430248

Attachments (3)

meta-20.before.PNG (92.8 KB) - added by SergeyBiryukov 4 years ago.
meta-20.after.PNG (77.3 KB) - added by SergeyBiryukov 4 years ago.
meta-20.patch (1.5 KB) - added by SergeyBiryukov 4 years ago.

Download all attachments as: .zip

Change History (31)

#1 @siobhyb
7 years ago

  • Component changed from General to wordpress.org/support

#2 @nacin
7 years ago

The issue is when they use <li> without <ul> and <ol> wrapping it. Longstanding problem; ideally bbPress is modified to balance these tags for us.

I imagine it might also be possible to adjust some CSS to prevent serious breakage. I've never seen such destruction from a missing <ol> or <ul> than I have on wordpress.org/support.

#3 @SergeyBiryukov
4 years ago

  • Keywords has-patch added

This was brought up in a recent Slack discussion.

meta-20.patch is an experimental patch that wraps standalone <li> tags in <ul>:

  • Code blocks are not affected, as they are htmlencoded earlier.
  • The filter runs on output, so user input is intact.

See before/after screenshots.

Test 1, from @netweb's Code Blocks & Ordered Lists

Input:

<li>
<ol>
first
</ol>
<ol>
second
</ol>
</li>

Output:

<ul><li>
<ol>
first
</ol>
<ol>
second
</ol>
</li>
</ul>

Test 2

Input:

<ul>
<li>Unordered list item
</ul>

<ol>

<li>Ordered list item
</ol>

<li>
Code block:
`
<li>List item inside code block
`

Output:

<ul>
<li>Unordered list item
</li>
</ul>
<ol>
<li>Ordered list item
</li>
</ol>
<ul><li>
Code block:
<pre><code>
&lt;li&gt;List item inside code block
</code></pre>
</li>
</ul>

This would need some unit test coverage, but the direction seems correct.

This ticket was mentioned in Slack in #bbpress by netweb. View the logs.


4 years ago

This ticket was mentioned in Slack in #forums by sergey. View the logs.


4 years ago

#6 @netweb
4 years ago

Related: #bbPress2357 Design Crashes! - Li button issue - Editor not filtering improper HTML usage.

#7 @netweb
4 years ago

#2809 was marked as a duplicate.

This ticket was mentioned in Slack in #meta by tellyworth. View the logs.


3 years ago

#9 @tellyworth
3 years ago

  • Keywords needs-testing added
  • Priority changed from low to high

#10 @SergeyBiryukov
3 years ago

  • Owner set to SergeyBiryukov
  • Status changed from new to accepted

This ticket was mentioned in Slack in #forums by clorith. View the logs.


3 years ago

#12 @obenland
2 years ago

#3710 was marked as a duplicate.

#13 @Otto42
2 years ago

#3720 was marked as a duplicate.

#14 @Otto42
2 years ago

#3793 was marked as a duplicate.

#15 @valentinbora
10 months ago

This is still an issue on bbPress 2.6.3 and meta [9437].

@SergeyBiryukov is the ticket still priority: high?

#16 follow-up: @SergeyBiryukov
10 months ago

  • Resolution set to fixed
  • Status changed from accepted to closed

In 9438:

Support Theme: Prevent standalone <li> tags from breaking the theme layout.

If a <li> tag is not preceded by <ul> or <ol>, prepend it with <ul> and let force_balance_tags() do the rest.

Fixes #20. See #bb2357.

#17 in reply to: ↑ 16 @dd32
10 months ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

Replying to SergeyBiryukov:

In 9438:

Support Theme: Prevent standalone <li> tags from breaking the theme layout.

If a <li> tag is not preceded by <ul> or <ol>, prepend it with <ul> and let force_balance_tags() do the rest.

Fixes #20. See #bb2357.

FWIW This seems to not work 100% with lists that have multiple items. I think the regex needs to either support a closing LI element preceding it or something like that.

This is the best example I could come up with: https://wordpress.org/support/topic/this-is-a-test-topic/#post-12379821
The three items should be part of the OL, but instead the second and third LI gets assigned to their own ULs (Which is probably better than the original problem)
To make it more annoying as an example, I opted to not include the closing </li> too :)

Last edited 10 months ago by dd32 (previous) (diff)

#18 @Clorith
10 months ago

  • Priority changed from high to normal

As the most pressing issue here has been tackled (the layout breaking completely), I feel we can lower the priority here in light of other more relevant tickets at this time.

#19 @dd32
10 months ago

#5011 was marked as a duplicate.

#20 @dd32
10 months ago

Perhaps we could tighten this up in a different way..

  • Match on <([ou]l)>(.*?)</$1> to separate it into lists and non-list content
  • Replace <li> with <ul><li> only outside of the list content matches

#21 @Cybr
10 months ago

<ol> tags seem to be broken at the moment. I'm not sure it's because of nesting tags.

Example POC:

<ol>
<li>Some entry.</li>
<li>Another entry with `code`.</li> <!-- this is transformed into a new <ul> wrap -->
</ol>

I also believe you can completely break the layout using <ol> with attributes like start and type, but I'm not sure if I should test that :)

#22 @geminilabs
9 months ago

  1. When I create an ordered list, only the first list element is ordered and the rest are unordered.
<ol>
  <li>Item 1</li>
  <li>Item 2</li>
  <li>Item 3</li>
</ol>

Becomes:

<ol>
  <li>Item 1</li>
</ol>
<ul>
  <li>Item 2</li>
  <li>Item 3</li>
</ul>
  1. When I add an ordered list inside an unordered list item, it becomes unordered.
<ul>
  <li>Item 1
    <ol>
      <li>Sub Item 1</li>
      <li>Sub Item 2</li>
    </ol>
  </li>
  <li>Item 2</li>
  <li>Item 3</li>
</ul>

Becomes:

<ul>
  <li>Item 1
    <ul>
      <li>Sub Item 1</li>
      <li>Sub Item 2</li>
    </ul>
  </li>
  <li>Item 2</li>
  <li>Item 3</li>
</ul>

https://wordpress.slack.com/archives/C02QB8GMM/p1582996447108500

#23 follow-up: @galbaras
9 months ago

How about taking the Trac ticket and/or Github approach and providing a preview? This way, posters will know what they're about to post and fix most of their own mistakes.

In the preview situation, the system just needs to protect against malice, instead of fixing the formatting for the poster.

#24 @geminilabs
9 months ago

@galbaras Great idea, I wish the WordPress forums provided a preview...

#25 in reply to: ↑ 23 @SergeyBiryukov
9 months ago

Replying to galbaras:

How about taking the Trac ticket and/or Github approach and providing a preview? This way, posters will know what they're about to post and fix most of their own mistakes.

This is being tracked in #153 and #3786, currently closed in favor of #bbPress2407.

#26 @galbaras
9 months ago

So why not use the suggestion from @netweb to use https://github.com/jawittdesigns/bbpress-live-preview until it's added into bbPress core (maybe, eventually, some day)?

#27 @dd32
9 months ago

Unfortunately https://github.com/jawittdesigns/bbpress-live-preview fatals upon activation, and I'm not sure of what the actual error is. If someone wants to test/patch/fix it we could consider its inclusion on the forums.

I'm going to make an attempt at fixing this shortly, but I'm expecting it'll miss an edge-case.

Nested lists are mostly unsupported and will be converted into non-nested lists instead, not ideal, but should be fine for a forum reply.

#28 @dd32
9 months ago

  • Resolution set to fixed
  • Status changed from reopened to closed

In 9601:

Support: Attempt to fix standalone <li> tags, take two.

This change operates a little differently to [9438] in which it operates on chunks of the content instead, to treat standalone <li>s differntly than those wrapped in <ul> or <ol> tags.

Unfortunately this doesn't support nested lists, and instead attempts to convert those to non-nested lists.

Fixes #20.

Note: See TracTickets for help on using tickets.