Closed (fixed)
Project:
Wysiwyg
Version:
7.x-2.x-dev
Component:
Editor - CKEditor
Priority:
Normal
Category:
Feature request
Assigned:
Unassigned
Reporter:
Created:
6 Apr 2015 at 19:39 UTC
Updated:
3 Nov 2020 at 20:45 UTC
Jump to comment: Most recent, Most recent file
Comments
Comment #1
elijah lynnComment #2
elijah lynnDoh! Previous patch didn't account for people who didn't have the Widget plugin enabled.
Comment #3
elijah lynnThis doesn't take into account items that are not supposed to be widgets. They still get initOn()'d. Working on a new patch.
Comment #4
elijah lynnNot sure how in the world to tell if something is a Widget yet. Posted on StackOverflow for now.
http://stackoverflow.com/questions/29636920/how-to-tell-if-an-element-is...
Comment #5
elijah lynnOkay, doesn't appear to be a way so I modified the CKEditor insert() abstraction to accept another parameter of 'isWidget'. I also moved the vars to the top to better meet Drupal coding standards.
Comment #6
elijah lynnReinmar posted an answer to my stack overflow question and the tldr; is that we should really use insertHTML if possible and the Widget initialization will take care of itself. I tested this by basically reverting #1927968: CKEditor InsertHtml method broken in webkit browsers because I saw in comment #8 (https://www.drupal.org/node/1927968#comment-8846299) that that fix is not needed for CKEditor 4.
I am wondering if we can remove all of this code or at least check the version of CKEditor in an if() block.
Comment #7
twodI'm not sure why anyone would ever insert CKEditor-widgets using Wysiwyg's API. Code using the API is not supposed to care which editor is attached and would thus not know anything about these widgets or how to format them. Any code/plugin which does know about CKEditor widgets is IMO better implemented as a CKEditor plugin, or directly against that API.
Anyway, we should probably call CKEditors'
insertHtml()as it does indeed work in 4.x. If that fixes widgets as a side-effect, good.Widget initialization is specific to CKEditor so I'm not prepared to change the API for that but, as you noted, we could just skip the whole workaround for CKEditor 4.x. (I'm not going to account exactly for in which version widget support was fixed since it's not [yet] included in the "Full" CKEditor package and because of the intended API usage.)
The attached patch works well with the latest CKEditor version when including the Widget and Placeholder plugins (currently not supported by Wysiwgy core so you'll need a 3rd party integration).
Btw, please don't post patches with incremental changes, it makes it really difficult to keep track of what to review and apply as issues grow. Always diff from 7.x-2.x and it'll help speed things up.
Comment #8
rv0 commentedPatch in #7 fixes it for me.
Thank you so much :)
My usecase was quite simple (just including it here for people who are googling on this issue):
Using Ckeditor 4.5 with the Image2 widget & Insert.module, it was impossible to edit the attributes of an inserted image without first saving the node.
As far as I'm concerned, this can go into dev. It won't break anything for people using Ckeditor 4 and Ckeditor 3 should continue to work fine.
Comment #9
rv0 commentedCan this go to dev please?
Comment #10
rv0 commentedPretty please?
Comment #12
twodWell, since you're so nice about it.... ;)
Apologies this took so long, completely missed there had been reviews, thank you for the nudge!
Comment #13
rv0 commented@TwoD
No problem, thanks!
Better late than never :)
Comment #15
michelleFYI in case anyone else runs into this problem.... This code can result in an infinite loop if any of the items don't have a type. I added a default case that does a skip++ and that solved it.