Make WordPress Core

Opened 12 years ago

Closed 10 years ago

#22799 closed defect (bug) (fixed)

Erase Formatting Doesn't erase

Reported by: rdall's profile RDall Owned by:
Milestone: 4.2 Priority: normal
Severity: normal Version: 3.4
Component: TinyMCE Keywords: upstream
Focuses: Cc:

Description

In the Content area of a page under the visual test manager of a page if you highlight text click the bold button add italics and cross out. When I I highlight the text and then click the erase format button it only removes the italics and leaves the cross out.

On two other builds I have tested this problem with it turns the text red?

Have tested on two local installs. (one clean to test this issue) and one external install. All using Trunk all the same issue. See screenshot for visual reference.

http://cl.ly/image/1Y1I1H2u471E

Attachments (2)

Screen Shot 2012-12-06 at 3.40.24 PM.png (127.6 KB) - added by RDall 12 years ago.
Screenshot of issue on two different builds.
22799.patch (969 bytes) - added by azaozz 12 years ago.

Download all attachments as: .zip

Change History (24)

#1 @nacin
12 years ago

Is this reproducible in 3.4.2?

@RDall
12 years ago

Screenshot of issue on two different builds.

#2 @helenyhou
12 years ago

  • Keywords needs-patch removed

That looks like two different themes to me, probably styling stuff (markup) differently. What's the HTML/Text tab show? Screenshot of that is probably fine.

#3 @helenyhou
12 years ago

  • Keywords reporter-feedback added

#4 @nacin
12 years ago

Related, #21037.

#5 @RDall
12 years ago

Yes I was able reproduce the problem in 3.4.2 It will remove the bold and the italics but the strike through was not removed. I also checked the html version and all tags were removed but the strike through tag.

http://cl.ly/image/1X1i2c1K0v0A

Last edited 12 years ago by RDall (previous) (diff)

#6 @nacin
12 years ago

  • Version changed from trunk to 3.4

@azaozz
12 years ago

#7 @azaozz
12 years ago

  • Keywords has-patch needs-testing added; reporter-feedback removed
  • Milestone changed from Awaiting Review to 3.6

Since we use <del> tag for the Strikethrough button, we need to specify it as removable with the Remove Format button. This can be set in the formats init option. Also added there few of the deprecated elements that are supported with the patched html5 schema.

#8 @azaozz
12 years ago

#21037 was marked as a duplicate.

#9 @MikeHansenMe
12 years ago

  • Cc mdhansen@… added

Tested the patch and it works as expected w/ 3.6-alpha.

#10 @SergeyBiryukov
12 years ago

#23359 was marked as a duplicate.

#11 @mark-k
12 years ago

HTML5 defines strong, del and em as having semantic meaning and not a styling one. If all the editor does is styling then it should use style="font-weight:bold" instead of strong etc..

Looking at the patch the deletion of strong, em, del, and ins is wrong as it changes the meaning of the content.

#12 @markjaquith
11 years ago

  • Milestone changed from 3.6 to Future Release

#13 follow-up: @azaozz
11 years ago

Yes, best to handle this when upgrading to TinyMCE 4.0. The above patch works but needs more (border cases) testing especially with more complex HTML.

For reference: there is a native contenteditable command editor.execCommand('removeFormat') that typically removes only inline styles. TinyMCE normalizes and extends this, so tags and classes can be removed too.

#14 @RDall
11 years ago

I'd love if we could get this committed in the trac ticket scrub of 3.7

#15 @RDall
11 years ago

Tested patch and it works as expected with 3.7 alpha. Is there any other specific things I would need to test to get this ticket closed?

#16 in reply to: ↑ 13 @SergeyBiryukov
11 years ago

Replying to azaozz:

Yes, best to handle this when upgrading to TinyMCE 4.0.

Related: #24067

#17 @RDall
11 years ago

FYI I tried this with the new TinyMCE in WordPress 3.9 and it still doesn't erase the strike through abet a minor annoyance but still there. Maybe we should report it to TinyMCE as a bug?

#18 @iseulde
10 years ago

  • Keywords needs-refresh added; has-patch needs-testing removed

Asked spocke to consider adding del. I'll keep this ticket updated. It'd be better if this is added to TinyMCE. In case we need to add it ourselves, the 2 year old patch needs a refresh. :)

#19 @iseulde
10 years ago

  • Keywords needs-refresh removed

Fixed in TinyMCE. :) So this will be fixed in WP soon.
https://github.com/tinymce/tinymce/commit/a8e99b7ee1d96817e2cdd1291609dcc62a531356

#20 @SergeyBiryukov
10 years ago

  • Milestone changed from Future Release to 4.2

#21 @iseulde
10 years ago

  • Keywords upstream added

#22 @iseulde
10 years ago

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

Fixed in [31700], see #31551.
Also happens fixed for ins now.

Note: See TracTickets for help on using tickets.