Opened 9 years ago
Closed 9 years ago
#11165 closed Bug (fixed)
The filebrowser plugin cannot be removed from the editor
Reported by: | Wiktor Walc | Owned by: | Marek Lewandowski |
---|---|---|---|
Priority: | Must have (possibly next milestone) | Milestone: | CKEditor 4.3.1 |
Component: | General | Version: | 4.0 |
Keywords: | Cc: |
Description
- Download the standard package
- Modify the replacebycode sample to include just a set of plugins:
CKEDITOR.replace( 'editor1', { plugins: 'enterkey,entities,toolbar,wysiwygarea,image', filebrowserBrowseUrl : '/ckfinder/ckfinder.html' } );
- Open the sample, press the Image button. See that the ""Browse Server" button is available, although the filebrowser plugin isn't really enabled.
The reason why is it happening is the way how the plugin interacts with the editor:
CKEDITOR.on( 'dialogDefinition'
is executed always when this plugin is merged into ckeditor.js (in release process). However, the init
method of a plugin is not executed, because it is not enabled.
One of possible solutions for this issue could be to check by the plugin itself inside CKEDITOR.on( 'dialogDefinition'
if I'm really enabled.
Change History (9)
comment:1 Changed 9 years ago by
comment:2 Changed 9 years ago by
Milestone: | → CKEditor 4.3.1 |
---|
comment:3 Changed 9 years ago by
Priority: | Normal → High |
---|---|
Status: | new → confirmed |
I agree with Olek that this may be extremely confusing.
comment:4 Changed 9 years ago by
Owner: | set to Marek Lewandowski |
---|---|
Status: | confirmed → assigned |
comment:6 Changed 9 years ago by
Status: | review → review_failed |
---|
The idea is good... some remarks about the implementation though:
- Couldn't "
!( 'filebrowser' in evt.editor.plugins )
" be replace with the more compact "!evt.editor.plugins.filebrowser
"? - Any chance to have a test for it?
comment:7 Changed 9 years ago by
Status: | review_failed → review |
---|
You're right about shortening this statement.
As for tests i didn't want to sacrifice too much time at the first time, however now i got quick idea how to write short TC. Unfortunately this test depends on other plugin. I've chosen flash, since image will be replaced with image2 soon.
Pushed to t/11165 dev and t/11165 tests.
comment:8 Changed 9 years ago by
Status: | review → review_passed |
---|
Rebased both branches.
- I pushed a very minor commit to dev branch to standardize the comment.
- I pushed a not very minor commit to tests:
- Fixed wrong
<title>
of the test HTML. - Used a generic dialog instead of flash dialog. This test should not depend on another, unrelated plugin which may mutate or even be removed in the future.
- Closed dialog after the assertion.
- Used
try,catch,finally
because in case of failure dialog would remain open and distort other tests.
- Used
- Fixed wrong
comment:9 Changed 9 years ago by
Resolution: | → fixed |
---|---|
Status: | review_passed → closed |
Masterized with git:cb474052e in dev and a5ae4293f in tests.
cc