WordPress.org

Make WordPress Core

Opened 15 months ago

Closed 10 months ago

Last modified 10 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 15 months 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 10 months ago.
tiny_mce_src-3.5.8-wp2.js (509.7 KB) - added by azaozz 10 months ago.
23337-3.5.patch (451.9 KB) - added by azaozz 10 months ago.

Download all attachments as: .zip

Change History (25)

jnicol15 months ago

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

comment:1 jnicol15 months ago

  • Cc jonathan@… added

comment:2 azaozz15 months 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: jnicol15 months ago

@azaozz 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.

Last edited 15 months ago by jnicol (previous) (diff)

comment:4 SergeyBiryukov15 months ago

  • Version changed from trunk to 3.5.1

comment:5 spocke15 months 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 spocke15 months 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 azaozz15 months 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 jnicol15 months 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 jnicol15 months 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: Problems not present. Results are consistent with the demo @spocke linked to, and the public Full demo on the TinyMCE website.

Version 0, edited 15 months ago by jnicol (next)

comment:10 spocke15 months 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 duchydog15 months 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 14 months ago by SergeyBiryukov (previous) (diff)

comment:12 desrosj14 months ago

  • Cc desrosj@… added

comment:13 azaozz14 months 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 14 months ago by azaozz (previous) (diff)

comment:14 follow-up: azaozz14 months 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 jnicol14 months ago

@spocke @azaozz Thank you!

comment:16 in reply to: ↑ 14 gorgoglionemeister12 months 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 nacin10 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 azaozz10 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.

azaozz10 months ago

comment:19 nacin10 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.tmw10 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 nacin10 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.