WordPress.org

Make WordPress Core

Opened 9 years ago

Closed 6 years ago

Last modified 6 years ago

#20304 closed defect (bug) (worksforme)

Strikethrough does not work for multiple paragraphs in the post editor

Reported by: tollmanz Owned by:
Milestone: Priority: normal
Severity: normal Version: 3.4
Component: TinyMCE Keywords:
Focuses: Cc:

Description

Overview

If you have multiple paragraphs of text in the text editor and select all of them and push the strikethrough button, the text appears to be striked through; however, upon publishing/updating, only the first paragraph of text retains the strikethrough.

Steps to Reproduce

1) In a new post write screen add two paragraphs of text.

2) Select all of the text

3) Click the strikethrough button

4) Viewing the HTML of the rich text editor through web inspector (note that I am not toggling the visual/html tabs) shows:

<del>
	<p>text</p>
	<p>text</p>
</del>

5) Save post

6) The text in the textarea only has strikeout for the first paragraph.

7) When viewing the source of the content on the front end, you get the following HTML:

<p>
	<del>text
</p>
<p>text</p>
<p>
	</del>
</p>

Other Information

I have tried these same steps on a clean install of 3.3.1. The strikethrough works as expected, and produces in all scenarios:

<p>
	<del>text</del>
</p>
<p>
	<del>text</del>
</p>


I first had this issue crop up for a .com client approximately 3-4 weeks ago, so this issue may have been introduced with a more recent patch.

Attachments (1)

20304-1.diff (1.1 KB) - added by jkudish 9 years ago.

Download all attachments as: .zip

Change History (11)

#1 @knutsp
9 years ago

  • Cc knut@… added

A part of the problem here may be that ins and del comes in two flavors, the inline elements and the block level elements of the same names. See http://www.w3.org/TR/html401/struct/text.html#edef-del. Depending on whether the elements appear at the block level or the inline level, they should ideally be treated differently.

The WordPress editors have never accepted ins/del as block level elements, but treats them as inline only elements. In 3.3.1, when you try to use them as block elements, they are "refactored" into several inline elements, one for each paragraph. This may be the expected behaviour, though.

Tested in 3.4-alpha-20275 and it works there as expected. Suggest worksforme.

#2 @tollmanz
9 years ago

In 3.3.1, the <del> tag would be refactored and inserted inline. This worked as expected. In 3.4 (I'm currently running 3.4-alpha-20284), the <del> tag is refactored and inserted incorrectly. I have had two other people verify this in the latest versions of trunk.

My concern is that this was working correctly and then the behavior was changed, which seems inconsistent with what someone would expect when using the strikethrough button.

@jkudish
9 years ago

#3 @jkudish
9 years ago

  • Cc joachim.kudish@… added
  • Component changed from Editor to Formatting
  • Keywords has-patch needs-testing added; needs-patch removed

Sitting next to @tollmanz and debugging with him. We figured out that the problem occurs on output and traced it to wpautop. Adding del to the list of elements in $allblocks seems to fix the issue. The proposed/attached patch does exactly that. Going to test this with a few different scenarios to make sure it's fine now.

#4 @jkudish
9 years ago

It seems that my patch fixed the front-end display with <del> but in the editor it's still shows up broken. Investigating further.

#5 @nacin
9 years ago

  • Milestone changed from Awaiting Review to 3.4
  • Severity changed from normal to blocker

Per comment 2, this is a regression from 3.3.

#6 @azaozz
9 years ago

  • Keywords needs-unit-tests added
  • Milestone changed from 3.4 to Future Release
  • Severity changed from blocker to normal

Don't think we can fix this at this moment as TinyMCE would not treat <del> as block level element in its cleanup. The next version comes with HTML 5.0 support, perhaps then it would work.

This used to work before as there weren't any <del> tags inserted in TinyMCE, instead they were <span style="text-decoration: line-through;">.

#7 @nacin
7 years ago

I think this is safe to revisit after not only the TinyMCE schema changes that dropped into core at the end of 3.5, but TinyMCE 4.0, which further improved HTML5 support.

These elements are called "transparent" in HTML5 — they can contain around blocks, or inline, but not both. Does adding it to allblocks mean it can still wrap inline text?

I think a patch and a unit test (and confirming that it works in TinyMCE, of course) would be sufficient for us to move forward on this.

#8 @nacin
7 years ago

  • Keywords wpautop added

#9 @iseulde
6 years ago

  • Component changed from Formatting to TinyMCE
  • Keywords has-patch needs-testing needs-unit-tests wpautop removed
  • Milestone Future Release deleted
  • Resolution set to worksforme
  • Status changed from new to closed

This seems to be fixed now. I cannot reproduce the issue. Feel free to reopen if I'm wrong.

This ticket was mentioned in Slack in #core by janneke. View the logs.


6 years ago

Note: See TracTickets for help on using tickets.