WordPress.org

Make WordPress Core

Opened 2 years ago

Closed 22 months ago

Last modified 22 months ago

#23337 closed defect (bug) (fixed)

TinyMCE, webkit and backspace/linebreak/italic issues

Reported by: jnicol Owned by: nacin
Milestone: 3.5.2 Priority: normal
Severity: normal Version: 3.5.1
Component: TinyMCE Keywords: fixed-major
Focuses: Cc:

Description

On WP 3.5.1 and nightly I notice irregularities in TinyMCE on webkit (Chrome, Safari).

When editing existing paragraphs in TinyMCE visual mode:

  • After deleting a paragraph break, it is impossible to insert a single linebreak
  • When deleting paragraph breaks, sometimes text will be wrapped in <em id="__mceDel">...</em>

Steps to reproduce:

  1. In a new post, type: "One[return]two[return]three" (i.e. three paragraphs)
  2. Position the caret before the word "three". Hit [delete] to merge paragraphs two and three.
  3. Do a shift+return key combo, which should insert a single linebreak. Instead, a paragraph break is inserted.
  4. Place the caret before the word "two". Hit [delete] to merge paragraphs one and two. The third paragraph is italicised. Viewing the HTML source will show that it has been wrapped in an em tag with the id "__mceDel"

Expected behaviour:

  • After deleting the space between two paragraphs, shirt+return should insert a single linebreak.
  • Text should not be italicised without the user having specified for TinyMCE to do so.

I can consistently reproduce this behaviour in Chrome and Safari. Firefox seems to work as expected.

I have tested with a clean install of WP 3.5.1 and the nightly build. I have tested on two different computers.

OS: OS X Lion
Browser: Chrome 24.0.1312.56, Safari 6.0.2

Attachments (4)

Screen Shot 2013-01-31 at 6.49.37 PM.png (10.2 KB) - added by jnicol 2 years ago.
Demonstration of incorrectly italicised text. This is step 4 in my list of steps to reproduce the bug.
tiny_mce_src-3.5.8-wp.js (509.3 KB) - added by azaozz 22 months ago.
tiny_mce_src-3.5.8-wp2.js (509.7 KB) - added by azaozz 22 months ago.
23337-3.5.patch (451.9 KB) - added by azaozz 22 months ago.

Download all attachments as: .zip

Change History (25)

@jnicol2 years ago

Demonstration of incorrectly italicised text. This is step 4 in my list of steps to reproduce the bug.

comment:1 @jnicol2 years ago

  • Cc jonathan@… added

comment:2 @azaozz2 years ago

  • Cc spocke added

WebKit has had problems with the backspace key in contenteditable mode for a long time... Trying to reproduce this, it actually inserts two <br> tags in step 3. Workaround for now is to delete one of them.

The <em id="__mceDel">...</em> seems to be a leftover from the internal logic in TinyMCE that corrects the broken backspace key in WebKit. It only happens when merging two paragraphs with the delete key and immediately entering a <br>.

Both if these problems will have to be fixed upstream.

comment:3 follow-up: @jnicol2 years ago

@azozz Thanks for looking at this. When you say "upstream", do you mean that the problem exists in TinyMCE independent of Wordpress, and would need to be fixed by the TinyMCE team?

When I compare WP's tinymce (v3.5.8-wp) with the demo on the TinyMCE website (http://www.tinymce.com/tryit/full.php - v3.5.8), the outcome is quite different.

I'm familiar with minor defects in the way TinyMCE handles backspaces in webkit - sometimes it can be hard to delete paragraphs (they keep reappearing lower in the document) but nothing as extreme as I'm experiencing in WP 3.5.1.

Version 2, edited 2 years ago by jnicol (previous) (next) (diff)

comment:4 @SergeyBiryukov2 years ago

  • Version changed from trunk to 3.5.1

comment:5 @spocke2 years ago

Couldn't reproduce it here:
http://www.tinymce.com/nightly/tinymce/examples/full.html

It properly inserts BR instead of P on Shift+Enter and merges the paragraphs correctly when using "backspace" not "delete". I would assume that's what you meant since "delete" wouldn't merge two and three in step 2.

This is what I did:

  1. Opened http://www.tinymce.com/nightly/tinymce/examples/full.html in Chrome 26.0.1397.2
  2. Selected all text using Ctrl+A and pressed "delete".
  3. Entered "one" + Enter, "two" + Enter, "three" + Enter.
  4. Placed the caret using the mouse before the word "three".
  5. Pressed "backspace" and observed that two and three got properly merged.
  6. Pressed shift+enter and observed that the paragraph got properly created.
  7. Placed the caret using the mouse before the word "two".
  8. Pressed "backspace" and observed that "one" and "two" got properly merged.
  9. Reviewed the source code by pressing the "html" button and saw no "em" anywhere.

comment:6 @spocke2 years ago

Ahh, heck. Just realized that our nightly build system haven't updated properly for a while. Jenkins tends to hang from time to time. Properly reproduced it now working on a fix.

comment:7 in reply to: ↑ 3 @azaozz2 years ago

Thanks @spocke.

Replying to jnicol: In trunk and WordPress 3.5.1 the version of TinyMCE is 3.5.8 + few patches (that's why it is shown as 3.5.8-wp). Most notably a fix for WebKit backspace that can remove all spans in some cases, see #23010.

comment:8 @jnicol2 years ago

@spocke I also can't reproduce the behaviour in the TinyMCE nightly build you linked to, or the Full example on the TinyMCE website. Only in Wordpress.

Perhaps I'm an edge case. I would imagine that if it was a widespread problem there would be more people making noise about it...

Sorry for confusion with "delete" key. I'm on an Apple Mac, which has a key labelled "delete" which is equivalent to a PC's "backspace" button.

comment:9 @jnicol2 years ago

@azozz @spocke I just did some further tests in older versions of Wordpress:

WP 3.5 stable: Linebreak behaviour consistent with 3.5.1, except that the "mceDel" italicization problem is not present.

WP 3.4 stable: Problems not present. Results are consistent with the demo @spocke linked to, and the public Full demo on the TinyMCE website.

Last edited 2 years ago by jnicol (previous) (diff)

comment:10 @spocke2 years ago

Ok, made a new hack for this annoying WebKit bug:
https://github.com/tinymce/tinymce/commit/cd84a63d4addf27c60ef32dcbe1a49bbee30150a

Seems to be working fine now and it also fixes a failing unit test we had for the enter key behavior. Though this doesn't solve Ctrl+Delete/Backspace since that would require us to write our own backspace/delete logic from scratch. Not sure I want to do that in a minor since that would probably take a week or so and introduce new bugs until it's ironed out. Will try to get the WebKit folks to fix this one since it's the only browser engine with this problem.

comment:11 @duchydog2 years ago

I am experiencing this problem on a new installation of WP 3.5.1 (using default theme). In Chrome (24.0.1312.57) for the Mac.

In this case, I pasted text into the content area using the plain-text setting, then switched back to the visual editor.

But now for that particular page (and that page only) the visual editor does not respond to the "delete" key until I press it twice, at which point it moves the cursor up instead of deleting text. However, if I use "enter," then the "delete" key, it inserts the tag em id="__mceDel" (I left out the <>s). As many times as I use the delete key, it inserts the tag (and closes it) into the post, as many as six or ten times.

Did not attempt any other troubleshooting.


Last edited 2 years ago by SergeyBiryukov (previous) (diff)

comment:12 @desrosj2 years ago

  • Cc desrosj@… added

comment:13 @azaozz2 years ago

  • Milestone changed from Awaiting Review to 3.5.2

Confirmed the patch by @spocke fixes all known quirks in WebKit. Lets test it in trunk for now and add to 3.5.2 before release.

Last edited 2 years ago by azaozz (previous) (diff)

comment:14 follow-up: @azaozz2 years ago

In 23402:

TinyMCE: include https://github.com/tinymce/tinymce/commit/cd84a63d4addf27c60ef32dcbe1a49bbee30150a to fix backspace and line breaks issues in WebKit, see #23337

comment:15 @jnicol2 years ago

@spocke @azaozz Thank you!

comment:16 in reply to: ↑ 14 @gorgoglionemeister2 years ago

Replying to azaozz:

In 23402:

TinyMCE: include https://github.com/tinymce/tinymce/commit/cd84a63d4addf27c60ef32dcbe1a49bbee30150a to fix backspace and line breaks issues in WebKit, see #23337

I'm sorry, how did you build this file from the sources? I would like to have a non-minified version of it to study, but the WordPress sources repository is updated up to 3.4.9.

comment:17 @nacin22 months ago

  • Keywords fixed-major added
  • Owner set to nacin
  • Status changed from new to accepted

Going to take a bit of finessing to merge this cleanly to the 3.5 branch — going to do this soon.

comment:18 @azaozz22 months ago

It should be straight replacement. The tiny_mce_src-3.5.8-wp.js is the non-minified version currently in 3.5.1. The tiny_mce_src-3.5.8-wp2.js​ is in current trunk and includes the 3 commits up to Feb 04, 2013.

@azaozz22 months ago

comment:19 @nacin22 months ago

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

In 24484:

TinyMCE: Fix backspace and line break issues in WebKit. Updates TinyMCE 3.5.8 to include 3 upstream commits.

props azaozz.
for the 3.5 branch.
see [23402] for trunk.

fixes #23337.

comment:20 @nikolov.tmw22 months ago

  • Cc ico.the.star.dust@… added

Correct me if I'm wrong, but I think this fix won't work for a lot of cases - since wp-tinymce.php which loads the tiny_mce.js file will load wp-tinymce.js.gz instead if compression is requested and available(I believe it's requested by default). And I didn't notice any changes in that file.

I was looking into bypassing this issue until it's fixed in the next release, but that seems not really possible(even if I replace the current tiny_mce.js with the one from the commit).

comment:21 @nacin22 months ago

You are correct, but we have a bot to handle this. See [24485/trunk] and [24486/branches/3.5].

Note: See TracTickets for help on using tickets.