WordPress.org

Making WordPress.org

Opened 7 years ago

Closed 2 weeks 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 3 years ago.
meta-20.after.PNG (77.3 KB) - added by SergeyBiryukov 3 years ago.
meta-20.patch (1.5 KB) - added by SergeyBiryukov 3 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
3 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.


3 years ago

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


3 years ago

#6 @netweb
3 years ago

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

#7 @netweb
3 years ago

#2809 was marked as a duplicate.

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


2 years ago

#9 @tellyworth
2 years ago

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

#10 @SergeyBiryukov
2 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.


2 years ago

#12 @obenland
21 months ago

#3710 was marked as a duplicate.

#13 @Otto42
21 months ago

#3720 was marked as a duplicate.

#14 @Otto42
19 months ago

#3793 was marked as a duplicate.

#15 @valentinbora
2 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
2 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
2 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 2 months ago by dd32 (previous) (diff)

#18 @Clorith
8 weeks 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
8 weeks ago

#5011 was marked as a duplicate.

#20 @dd32
8 weeks 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
6 weeks 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
4 weeks 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
4 weeks 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
4 weeks ago

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

#25 in reply to: ↑ 23 @SergeyBiryukov
4 weeks 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
4 weeks 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
2 weeks 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
2 weeks 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.