Opened 8 years ago

Closed 7 years ago

#3593 closed Bug (fixed)

Use body.contentEditable = true instead of domDocument.designMode = 'on'

Reported by: Alfonso Martínez de Lizarrondo Owned by: Garry Yao
Priority: Normal Milestone: CKEditor 3.3
Component: General Version: SVN (CKEditor) - OLD
Keywords: IBM Confirmed Review+ Cc: satya_minnekanti@…

Description

Except Firefox 2, the rest of the browsers support contentEditable, and their behavior should be almost the same with at least one big difference:

If body.contentEditable=true is used then the behavior of elements inside the body with contentEditable=false is as expected and the browser itself takes care of not allowing to edit them. This is really useful for little things like the Placeholder (v2 only for the moment) plugin.

This behavior is reported in https://bugzilla.mozilla.org/show_bug.cgi?id=462735 , https://bugs.webkit.org/show_bug.cgi?id=22036 and in the internal Opera tracker.

Attachments (4)

3593.patch (1.5 KB) - added by Alfonso Martínez de Lizarrondo 8 years ago.
Proposed patch
3593_2.patch (1.9 KB) - added by Garry Yao 8 years ago.
3593_3.patch (1.6 KB) - added by mattis 8 years ago.
Updated patch to work with latest SVN revision of WYSIWYG plugin
3593_4.patch (3.7 KB) - added by Garry Yao 8 years ago.

Download all attachments as: .zip

Change History (22)

comment:1 Changed 8 years ago by Garry Yao

Keywords: Confirmed added

It's really a nice proposal, none-editable element will be needed sooner or later.

comment:2 Changed 8 years ago by Frederico Caldeira Knabben

Milestone: CKEditor 3.1

Changed 8 years ago by Alfonso Martínez de Lizarrondo

Attachment: 3593.patch added

Proposed patch

comment:3 Changed 8 years ago by Alfonso Martínez de Lizarrondo

Keywords: Review? added
Owner: set to Alfonso Martínez de Lizarrondo
Status: newassigned

comment:4 Changed 8 years ago by Garry Yao

Keywords: Review- added; Review? removed
Type: New FeatureBug

The patch doesn't work in sense of the cursor is not blinking while focus is on the document instead of body, reproduced by the following step:

  1. Left click outside the editor chrome;
  2. Press Tab to give focus to the editor;
    • Actual Result : The cursor is not blinking.

comment:5 Changed 8 years ago by Garry Yao

Providing a patch based on Alfonsoml's while targeting the above issue.

Changed 8 years ago by Garry Yao

Attachment: 3593_2.patch added

Changed 8 years ago by mattis

Attachment: 3593_3.patch added

Updated patch to work with latest SVN revision of WYSIWYG plugin

comment:6 Changed 8 years ago by mattis

We found some discrepancies between contentEditable in IE and FF:

In FF 3.5.3:

  • Able to move caret into the span when using arrow keys
  • Able to press enter inside the span

We did some research how to stop being able to hit Enter inside the span:

CKEDITOR.on('instanceReady', function(editorEvent){
	editorEvent.editor.on('key', function(keyEvent) {
	    var node = keyEvent.editor.getSelection().getStartElement();
	    
	    if(node.getAttribute('contentEditable') == 'false') {
	        keyEvent.cancel();
	    }
	});
});

This cancels all keyEvents inside spans with contentEditable false, but also traps the cursor inside the span, so it needs some more tweaking.

comment:7 Changed 8 years ago by Garry Yao

Keywords: Review? added; Review- removed

@mattis: Thanks for the bug sharing while this ticket is only dedicated in enable contentEditable on body element.

comment:8 Changed 8 years ago by Frederico Caldeira Knabben

Keywords: Review- added; Review? removed
Milestone: CKEditor 3.1CKEditor 3.2
  • In FF3.5, the following TC makes the caret disappear:
  1. Load replacebyclass.
  2. Hit ENTER at the end of the first line.
  3. Type "Test" in the new line.
  4. Click in the first line to move the caret.
  5. Click in the white space after the full text.

There are several other ways to have this issues reproduced but the above looks like a simple TC that always happen.

This issue may be due to the fact that the <body> is editable and we're clicking outside of it (just like the famous IE bug for the focus on click).

I was able to fix this issue by setting body.parent.contentEditable instead of body.contentEditable, but the impact of it must be checked on other browsers.

  • The same problem with Opera. The patch makes it behave just like IE currently.
  • In FF2, the caret blinks inside the editing area, it's not possible to type text.
  • The patch disables the hack described at line 368.
  • Let's still check if it's possible to find a way to make it work without the designMode hack for FF. Maybe a change on the way the focus is set.

comment:9 Changed 8 years ago by Alfonso Martínez de Lizarrondo

Owner: Alfonso Martínez de Lizarrondo deleted
Status: assignednew

comment:10 Changed 8 years ago by Frederico Caldeira Knabben

Milestone: CKEditor 3.2CKEditor 3.3

comment:11 Changed 8 years ago by Damian

Keywords: IBM added

comment:12 Changed 8 years ago by Frederico Caldeira Knabben

Milestone: CKEditor 3.3CKEditor 3.x

comment:13 Changed 8 years ago by Damian

Could we provide this feature as an optional config for products not relying on FF2. If the patch works with all browsers that support contentEditable, it would be a valuable thing to switch this on.

We have a few teams eagerly waiting for this support and are prepared for it not to behave the same in FF2.

comment:14 Changed 8 years ago by Frederico Caldeira Knabben

Milestone: CKEditor 3.xCKEditor 3.3

Ok, let's have it in the 3.3, but not on the 3.2.x.

comment:15 Changed 8 years ago by Satya Minnekanti

Cc: satya_minnekanti@… added

Changed 8 years ago by Garry Yao

Attachment: 3593_4.patch added

comment:16 Changed 8 years ago by Garry Yao

Keywords: Review? added; Review- removed
Owner: set to Garry Yao
Status: newassigned

This patch tries to make contentEditable works in the same way as designMode in all browsers.

comment:17 Changed 7 years ago by Frederico Caldeira Knabben

Keywords: Review+ added; Review? removed

Let's watch the editor behavior with attention after this fix. Please commit it into the 3.3 branch.

comment:18 Changed 7 years ago by Garry Yao

Resolution: fixed
Status: assignedclosed

Fixed with [5391] on 3.3.x branch.

Note: See TracTickets for help on using tickets.
© 2003 – 2017 CKSource – Frederico Knabben. All rights reserved. | Terms of use | Privacy policy