In the interest of having all libraries in the same area and to bring continuity to how the CKEditor Module finds the CKEditor library, I would like to see the ability to add the CKField library be able to be loaded from sites/all/libraries.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

mikeskull’s picture

Is there any movement on this, ive managed to get this working by hacking the module and messing with the ckeditor.lib.inc file to make it point at sites/all/libraries if ckfinder exists inside it use that if not fallback and use the module path as normal, but would be nice to get it working in the correct way.

dczepierga’s picture

Status: Active » Postponed

We will try to release this feature with the next verison of CKEditor module.
So pls be patient...

Greetings

rooby’s picture

Title: Allow CKFinder to be placed in sites/all/libraries » Use Libraries API for ckeditor and ckfinder
Version: 7.x-1.0 » 7.x-1.x-dev
Status: Postponed » Active
Issue tags: -ckfinder +ckfinder Libraries

This module should integrate with the Libraries API module. Then it will be also covered for multi site installs and install profiles etc.

hadsie’s picture

Status: Active » Needs review
FileSize
6.08 KB

Using the actual libraries API would definitely be a better way of doing this. Here's a patch that replaces most of the sites/all/libraries hardcoded paths with a call to libraries_get_path if it exists. Falls back to sites/all/libraries so it's backwards compatible.

helior’s picture

FileSize
6.48 KB

Re-rolling patch to accommodate for changes made to CKeditor since #4 was posted.

roderik’s picture

FileSize
6.4 KB

Attached patch is the same as #3/#4 but it adds some comments, which I added while trying to understand what's up with this ticket.

A summary:
- the functionality requested in the original issue/#1 has been incorporated in the -dev version already, as per #1349330-2: [D7] Add configuration option to set CKFinder library path / http://drupalcode.org/project/ckeditor.git/commitdiff/ab2243101742747382...

- Then #3 talks about using the Libraries API instead of hardcoding 'sites/all/libraries'. This is done in the next patches.

===

There is actually something strange with this, because now there are two interdependent config options:

* the libraries API (trying to find CKEditor whereever it is)
* the ckeditor.module admin screen, which gives user-configurable options

This indeed gives extra flexibility, but keep in mind the following strangenesses:

1)
The user-configurable '%l' which should be used when CKEditor is inside a libraries directory, actually changes with the location where CKEditor is installed. This means that if you use '%l', it makes absolutely no sense to use anything else than '%l/ckeditor'.

2) the possible locations where CKFinder can be found, depend on the location where CKEditor is installed.

ezra-g’s picture

Status: Needs review » Needs work

These patches no longer apply.

gaspaio’s picture

Will this issue be closed any time soon ?
The patch should be commited when is does work for the dev version ; if not, i don't see the point in updating patches that will never be commited.
If the module maintainers assure us that a working patch WILL be commited when it's ready and working, i'll gladly update the one above against the current DEV.

nod_’s picture

I'll commit it when it's ready, it's needed for the edit module.

nod_’s picture

Status: Needs work » Needs review
FileSize
1007 bytes

Seems to be enough to make it work, please test.

webchick’s picture

Status: Needs review » Needs work

Spark is using this patch, but CKEditor still not working. :(

http://spark7.localhost:8082/profiles/spark/modules/contrib/ckeditor/ckeditor/ckeditor.js?mgy07b&_=1358716597647
Failed to load resource: the server responded with a status of 404 (Not Found)

So it's still looking in profiles/spark/modules/contrib/ckeditor, even though the library was downloaded to profiles/spark/libraries/ckeditor.

nod_’s picture

Some more context, this is using the spark distribution (how-to available here for install #1879820-11: [meta] Get the D7 version of Edit production-ready) and editing the teaser text on the front page.

The issue is that window.CKEDITOR_BASEPATH is set to the wrong value.

nod_’s picture

Status: Needs work » Needs review
FileSize
2.98 KB

that fixes the issue above but I don't know what else I broke in the process.

nod_’s picture

woops forgot to remove a change

wwalc’s picture

Status: Needs review » Needs work

With patch from comment (#14) Spark was still unable to find CKEditor. The reason is the order of installed modules in the spark profile. CKEditor is installed first, then the libraries module. So, the path to CKEditor library was not resolved properly during installation and invalid value was set in the "Global Profile".

I think that was even the reason why nod_ had
$global_profile->settings['ckeditor_path'] = '%l/ckeditor';
in previous patch.

To fix the issue with discovering path to CKEditor we need to:
1. Use hook_modules_enabled() to update paths to CKEditor once Libraries API is enabled.
2. Fix ckeditor_library_path() used by _ckeditor_script_path() which is the main function that finds ckeditor.js (and detects the location of the editor)

wwalc’s picture

nod_’s picture

Status: Needs work » Needs review

That works well for me, need to disable and enable the libraries modules then it works fine :) Thanks!

I'm not sure how it'd impact people using less-default configuration than me though. As far as I'm concerned it's RTBC.

wwalc’s picture

nod_’s picture

for those with already installed ckeditor modules, you need to disable and enable the libraries module.

Thanks a lot! will make it easier on spark distrib :)

ezra-g’s picture

Title: Use Libraries API for ckeditor and ckfinder » Use Libraries API for ckeditor
Wim Leers’s picture

Title: Use Libraries API for ckeditor » Use Libraries API for CKEditor
Peter Bowey’s picture

Title: Use Libraries API for ckeditor » Use Libraries API for CKEditor

webchick’s picture

Issue tags: +Edit D7 Backport

Just retro-actively tagging as part of the D7 backport of Edit module; don't mind me. :D

Simon Georges’s picture

Status: Fixed » Needs review
Issue tags: -ckfinder Libraries +ckfinder
FileSize
734 bytes

There still is a bug, during installation of the module. I've just moved up the module_load_include up so the _ckeditor_requirements_isinstalled() function can be called.

Simon Georges’s picture

oups, retagging.

jcisio’s picture

Status: Needs review » Fixed

Revert as there is another issue for that #1898294: Fatal error if Libraries module is enabled after CKEditor which is RTBC ;)

Status: Fixed » Closed (fixed)

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

saltednut’s picture

Status: Closed (fixed) » Needs work

Really sorry to re-open this but I don't think this work is complete.

I'm using an install profile and I have ckeditor installed under profiles/[distroname]/libraries/ckeditor

This works fine with Edit module (inline editing) only.

If I try to use ckeditor inside a regular node/#/edit screen, I see the following error:

Failed to load resource: the server responded with a status of 404 (Not Found) http://localhost/profiles/df/modules/contrib/ckeditor/ckeditor/ckeditor....

It should be looking for ckeditor in the libraries folder only. There is no point in having it support putting the library in the module folder - this is not a best practice. If the module was currently under review to be promoted from a sandbox to full project, that would be a blocker.

I know its a ton of refactoring, but stripping out the "old way" and moving over to full libraries API integration with no support for the library in the modules/ckeditor/ckeditor folder has many advantages:

1. Cleaner code
2. Common API Usage
3. Ease of installation

nod_’s picture

I don't think anyone here questions that.

But:
1) time, you said it, it'll be a pain to fix
2) upgrade path for the 154742 (!) D7 installs of ckeditor

saltednut’s picture

My problem stemmed from the settings page where I needed to use the %l token instead of the %m token. I have it working now but didn't find that easily.

Anyway, I'm not sure if the issue should stay open. Maybe if someone wants to hack away at this they should file a follow-up?

jcisio’s picture

Status: Needs work » Closed (fixed)

Yes, it's painful removing support for the library inside module folder. However, if CKEditor does not consistently detect the library folder, then it is a bug that needs fix, in a separate issue.