Opened 11 years ago
Closed 10 years ago
#11478 closed Bug (fixed)
Can't use jQuery Object as option-property with the jQuery Adapter
Reported by: | Encaitar | Owned by: | Artur Delura |
---|---|---|---|
Priority: | Normal | Milestone: | CKEditor 4.4.2 |
Component: | General | Version: | 4.3.2 |
Keywords: | Cc: |
Description
You can't use jQuery objects as options when using the adapter:
$( '#editable' ).ckeditor( function() { /* Callback function code. */ }, { uiColor : '#9AB8F3', test: $('body') //Results into a Uncaught TypeError: Illegal constructor }); // Use CKEDITOR.inline().
I was upgrading from 3.6 to 4.3 when I got this problem. I used those jQuery objects in my company's plugins. For more info, see: http://stackoverflow.com/questions/21159229/using-own-properties-in-ckeditor-4-plugin-upgrade-from-version-3
In the attached file is a ckeditor_4.3.2_full.zip with one 'bugged' example -> jquery_object.html
Attachments (1)
Change History (11)
Changed 11 years ago by
Attachment: | ckeditor_4.3.2_full.zip added |
---|
comment:1 Changed 11 years ago by
Owner: | set to Artur Delura |
---|---|
Status: | new → review |
The problem is that in commit https://github.com/ckeditor/ckeditor-dev/commit/aeff33d4c0e346e674d8ba4d1f7c0faeeb021217 was added copy configuration functionality, which simply don't work with jQuery objects. Above functionality use CKEDITOR.tools.clone
function. You pass jQuery object which under "0" property contains a value which is HTMLBodyElement. While copying HTMLBodyElement
we simply try to construct it which fail with error Illegal constructor
.
I proposed solution in branch:t/11478 which is wrapping construction call in try catch.
comment:2 Changed 11 years ago by
A try{}catch{} block will suppress future errors. I propose the alternative "if(obj instanceof jQuery)"
comment:3 Changed 11 years ago by
Thanks funeworks!
And my reply:
https://github.com/cksource/ckeditor-dev/pull/2#issuecomment-43179532
We need to do a research what can we clone. I think that now we try to clone simply too much.
comment:4 Changed 11 years ago by
So detecting for jQuery (or any specific object) isn't the solution, you're quite right. I'll watch this thread to see how the story develops. Not a major issue for our installation, we're sticking to an older release for the time being, but curious to see how/if this affects others.
comment:5 Changed 11 years ago by
Status: | review → review_failed |
---|
I've forgotten to R- the solution proposed in commit:1. Such usage of try-catch will only hide problems rather than fix them.
comment:6 Changed 11 years ago by
Status: | review_failed → review |
---|
Now we ommit DOM objects and window when copying config. So mentioned above are passed by reference. SOlution proposed in branch:t/11478b
comment:7 Changed 11 years ago by
Looks great! Sorry if this is a silly question but, when .... well, how do I find out when this particular commit has made its way into a stable release?
comment:8 Changed 11 years ago by
I see that this ticket has not milestone yet, so we'll review it and if it will be good (it looks ok), then it will make it to next minor release (i.e. 4.4.2).
comment:9 Changed 10 years ago by
Milestone: | → CKEditor 4.4.2 |
---|---|
Status: | review → review_passed |
I rebased branch:t/11478b and pushed additional commit with some small fixes.
comment:10 Changed 10 years ago by
Resolution: | → fixed |
---|---|
Status: | review_passed → closed |
Merged to master with git:c41e10c
CKEditor 4.3.2 full version, with my 'bugged' example (in the example folder)