In the plugin for CKEditor is an insert method that leverages CKEditor's insertHtml method;

  insert: function(content) {
    content = this.prepareContent(content);
    CKEDITOR.instances[this.field].insertHtml(content);
  }

Because it relies on the DOM implementation, this method doesn't work correctly in webkit browsers (see this ticket). Any wrapping elements are stripped, and replaced with a span containing styling information (for me this was a line-height).

This manifested itself for me when using Node Embed 7.x-1.1 in Chrome 25.0.1364.99, but the issue would equally apply to any module that used this method. I've implemented the fix from the linked ticket in my local version, and it seems to fix the issue;

  insert: function(content) {
    content = this.prepareContent(content);
    CKEDITOR.instances[this.field].insertElement(CKEDITOR.dom.element.createFromHtml(content));
  }

See also attached patch.

CommentFileSizeAuthor
#7 wysiwyg-ckeditor-insert.1927968.7.patch1.34 KBTwoD
#4 wysiwyg-ckeditor-inserthtml-1927968-4.patch505 bytesDavid_Rothstein
insert_patch.patch490 bytesben.kyriakou
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

dagmar’s picture

dagmar
Primary language Spanish
Location Argentina
CreditAttribution: dagmar commented
Status: Active » Needs review
TwoD’s picture

TwoD
Primary language Swedish
Location Sweden
CreditAttribution: TwoD commented

Hmm, that seems to break IE 6 and 7. They throw errors or just break paragraphs when trying to insert a Teaser Break placeholder image, or anything else.

TwoD’s picture

TwoD
Primary language Swedish
Location Sweden
CreditAttribution: TwoD commented
Status: Needs review » Needs work

Looks like we either need to find another workaround or wait for the result of the CKEditor ticket to see what they come up with. Or perhaps wait for a webkit fix.

David_Rothstein’s picture

David_Rothstein CreditAttribution: David_Rothstein commented
Version: 7.x-2.2 » 7.x-2.x-dev
FileSize
wysiwyg-ckeditor-inserthtml-1927968-4.patch505 bytes

Here's a reroll that applies using Git.

This patch is so far working wonders for me for some odd bugs seen using the WYSIWYG Fields module in Chrome. So it seems to be a good option (if you don't care about IE6 and IE7 support on a particular site).

TwoD’s picture

TwoD
Primary language Swedish
Location Sweden
CreditAttribution: TwoD commented

Related CKEditor ticket: http://dev.ckeditor.com/ticket/8227

I'm still a bit nervous of dropping IE7 support at this time...

TwoD’s picture

TwoD
Primary language Swedish
Location Sweden
CreditAttribution: TwoD commented
Issue summary: View changes

Added more information on the issue manifested

mgifford’s picture

mgifford
he/him
Primary language English
CreditAttribution: mgifford commented
Issue summary: View changes

Is it a question of dropping IE7 or Chrome? Seems Chrome isn't dealing with the bug (or at least making any progress on the code). Inserting a bunch of bad html in the output seems to be a good reason to either make this switch or warn users about using Chrome.

TwoD’s picture

TwoD
Primary language Swedish
Location Sweden
CreditAttribution: TwoD commented
Status: Needs work » Needs review
FileSize
wysiwyg-ckeditor-insert.1927968.7.patch1.34 KB

Let's just change things for WebKit (and forks of it since they still suffer the same issue).
The previous patch did not work when inserting multiple elements in one insert() call, and CKEditor does not seem to accept a document fragment for insertElement(), so let's use a temporary div for generating the elements and then insert each child one by one. The caret is automatically positioned after each inserted element so this works fine.

Tested in Chromium, Opera and Safari.

TwoD’s picture

TwoD
Primary language Swedish
Location Sweden
CreditAttribution: TwoD commented
Status: Needs review » Fixed

This is now in 7.x-2.x and 6.x-2.x. CKEditor 4 does apparently not need this workaround, but it doesn't hurt having it there.

Thanks all!

  • Commit faadd64 on 6.x-2.x by TwoD: 
    - #1927968 by TwoD: Fixed CKEditor insert method in WebKit browsers.
  • Commit 359b8ea on 7.x-2.x by TwoD:
    - #1927968 by TwoD: Fixed CKEditor insert method in WebKit browsers.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.

eyilmaz’s picture

eyilmaz
Primary language German
Location Berlin, Germany
CreditAttribution: eyilmaz commented
Status: Closed (fixed) » Needs work

Wysiwyg fields(image field) with Ckeditor 4.4.5 breaks in Chrome because of this workaround.
Please apply this Workaround only for CKEditor 3 if possible

Greetz
eyilmaz

TwoD’s picture

TwoD
Primary language Swedish
Location Sweden
CreditAttribution: TwoD commented
Status: Needs work » Postponed (maintainer needs more info)

Please elaborate on "breaks". I can't make sense of what you're trying to say. Image fields do not get WYSIWYG editors applied to them.

eyilmaz’s picture

eyilmaz
Primary language German
Location Berlin, Germany
CreditAttribution: eyilmaz commented

Ok.

I'm using this Module with CKEditor 4.4.5 and the Module WYSIWYG Fields

I configured an image field to appear as a wysiwyg field by activating "Attach to Wysiwyg?".

When I add an Image Field by click on the image-field button & uploading an image & insert
following html is inserted to Ckeditor:

On Firefox:

<img alt="" height="208" src="Path_to_image" width="250" />

On Chrome (or any other webkit browser):

<wysiwyg_field contenteditable="false" wf_deltas="0" wf_field="field_name" wf_formatter="image" wf_settings-image_link="" wf_settings-image_style=""><img alt="" height="162" src="path_to_image" width="120" /></wysiwyg_field>

When I disable this lines and only use
CKEDITOR.instances[this.field].insertHtml(content);
evertything is working as expected.

TwoD’s picture

TwoD
Primary language Swedish
Location Sweden
CreditAttribution: TwoD commented

After looking through the WYSIWYG Fields module's code, I think what now gets inserted into the webkit based browsers is correct. The module uses the <wysiwyg_field....></wysiwyg_field> wrapper to determine which elements have been inserted by it (See http://cgit.drupalcode.org/wysiwyg_fields/tree/scripts/wysiwyg.js), and to prevent the user from messing with it using contenteditable="false".

I haven't tested this yet, but I assume the wrapper will be removed at some point by the module. Maybe the module tried to compensate for the old bug where wrappers would be removed, but that's no longer needed, or something like that.

Either way, I'll need to dig deeper to know where the real problem is.

eyilmaz’s picture

eyilmaz
Primary language German
Location Berlin, Germany
CreditAttribution: eyilmaz commented

Hi,

I understand now the behaviour.
On wysiwyg detach it should convert the HTML with the wrapper to tokens (see here), which doesn't happen because the detach function of the plugin is not called ?!

I think it's a fault of WYSIWYG_Field and since the Maintainer of WYSIWYG_Fields has clearly stated that WYSIWYG_Fields doesn't support CKEditor 4 (see here), there is nothing todo..

thank you

TwoD’s picture

TwoD
Primary language Swedish
Location Sweden
CreditAttribution: TwoD commented

Ah, probably related to the big changes introduced in CKEditor 4. I would guess that might the same for TinyMCE 4 as the module seems to have very specific code for both editors.

Elijah Lynn’s picture

Elijah Lynn
Location Portland, Oregon
CreditAttribution: Elijah Lynn at NBCUniversal commented

As a result of this, Widget initialization does not happen. I created a new issue here -> https://www.drupal.org/node/2466297