Adding ckeditor.make can simplify the library download when integrating with Drupal installation profile .make.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

hswong3i’s picture

hswong3i’s picture

Replace download link as reference to #2169401: Provide direct download link for the necessary custom build of CKEditor:

http://download.cksource.com/CKEditor%20for%20Drupal/edit/ckeditor_4.3.2_edit.zip
hswong3i’s picture

Update as:

http://download.cksource.com/CKEditor%20for%20Drupal/edit/ckeditor_4.3.3_edit.zip
wwalc’s picture

The "edit" version of CKEditor is no the best package for this module alone as it contains both the sourcearea and sourcedialog plugins which will result in duplicate Source buttons. Would you mind if I go ahead with the full version that you had in the first patch?

hswong3i’s picture

From pre-D8 UX improvement point of view, I would like to recommend using CKEditor in D7 together with Edit (https://drupal.org/project/edit); BTW, Edit require for this custom build version of CKEditor ;-)

New patch for v4.3.4.

wwalc’s picture

Humm I'm in a quite unfortunate position where whatever decision I make, it's going to have some bad consequences. Right now the module uses the CDN version of CKEditor by default, the "full-all" version of CKEditor (for the best flexibility), which basically has everything (sharedspace, sourcedialog, but also tableresize, autogrow etc.). This is perhaps not the smartest solution in terms of performance (especially that each "extra" plugin that is not merged into ckeditor.js results in an additional HTTP reguest), but provides everything out of the box. If one requires faster, smaller editor, he can download it by himself.

If ckeditor.make is configured to download a different version of CKEditor module (the "edit" version), then UX will be bad because of possible WTF of a user who got different solution depending on whether drush was used or not.

The full-all option would fit both cases, at least in theory... but it is not offered for download on the CKEditor download page.

wwalc’s picture

Status: Needs review » Fixed

Considering the problem from the last paragraph (with the lack of easily downloable full-all package), I'm going to use the edit version as suggested here (which is basically: full preset + sharedspace + sourcedialog).

Thanks!

// If anyone has any suggestions, please feel free to comment.

  • Commit 59627d8 on 7.x-1.x authored by hswong3i, committed by wwalc:
    Issue #2175565 by hswong3i: Download Library with ckeditor.make for...

Status: Fixed » Closed (fixed)

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

saltednut’s picture

Status: Closed (fixed) » Active

Is this always going to use the "CKEditor for Drupal" that is guaranteed to work with Edit? I am guessing no. Seems ripe for misconfiguration. Someone down the line will have to remember this thread when updating this module that we need the 'CKEditor for Drupal' version.

TBH, I don't find this to be useful at all for "easy profile building" -- what if my profile needs to run with a specific version of CKEditor?

If that happens, I have to patch this OUT of the module for the profiles I maintain. The pattern of including make files with modules seems like a good idea on paper but it makes it hard to track down bugs and actually creates more work for everyone in the end. You end up having to nurse your builds and rely on script macros that pick things up along the way. So much easier to maintain things in a single make file.

It also leads to difficult decisions of the maintainer, just as it was mentioned before. What version to include? Not everyone wants the same build.

My biggest gripe here is that this was simply committed with no community review at all. Two people worked on this issue - a single distro maintainer who wants it and the module maintainer who didn't consider the consequence, imo.

Might have been good to clue in other distro maintainers that this thread existed, I would have voted against this patch and un-RTBC'd it until eternity. :)

Finally, what happens when CKEditor gets updated? Do we assume this module will get 0-day release for new versions? Seems a lofty goal.

hswong3i’s picture

Before acting as distribution maintainer, we are all module user.

As a module user, I am always confused with:

  • Which specific version of library shall I choose when using with this module?
  • Is this specific version of library fully tested by the module maintainer, so it will not coming with runtime functionality conflict?

IMHO, it is good to clue in other module users that also confuse with above issue, but not other distribution maintainers (yes, although I start this thread as a distribution maintainer).

Moreover, as wwalc mentioned, latest version of CKEditor module already coming with ability of using CDN version directly, which means both "CDN version" and "CKEditor for Edit version" are verified as suitable and stable for normal module user use cases, therefore it can introduce to normal user with default .make.

0-Day release for new version? Shall we first test it as a module maintainer point of view, verify if conflict happened, solve any potential bugs, before introduce into its own .make, and so recommend for general module user?

If we hope to have a new version of CKEditor included, should we contribute THIS module and so make sure no trouble introduce to normal non-distribution module user, but not only apply changes to our own distribution?

Before introduce local customization to down-stream projects, please kindly consider contribute changes to up-stream project so benefit for everyone ;-)

saltednut’s picture

The problem of "what to use" could be easily solved with a README that is kept up to date for "normal" users. The issue specifically pointed toward providing the make file for distributions. Now you change the argument saying its best for everyone? I honestly don't see how this is helpful for anyone. That said, since its currently set to the version of CK that we need, I won't argue that this should be reverted even though I do not find it helpful. It's really just another thing we'll have to account for.

jcisio’s picture

Title: Download Library with ckeditor.make for Simple Profile Integration » (revert) Download Library with ckeditor.make for Simple Profile Integration
Status: Active » Needs review

Well, I just tested the latest CKEditor snapshot in Scald Galaxy. The edit build shipped with CKEditor is downloaded and then replaced with the custom build in the installation profile make file. So, there is no need to patch ckeditor.make.

However, the CKEditor library is downloaded twice, which is not necessary. Like brantwynn, I don't think this makefile is useful for profile maintainers. It is quite easy to include a build that they want, instead of being forced by a module.

This makefile is never intended for normal user. There is already a drush command to download the library (which is now no longer recommended because it downloads CKEditor 3.x, but it should be a separate issue).

So, I think it should be reverted and we need an issue to take care of the drush integration. I'm marking as NR to have feedback, even there is no patch.

PS: about zero day, I think it mean the won't be a new CKEditor module release to update that make file each time the CKEditor library has a new version.

wwalc’s picture

@jcisio - I'll rely on your opinion here. The patch was waiting here for more than three months without any negative feedback, so I did not see any issues with it by myself. I personally have never worked with profiles / distributions, so my knowledge here was definitely limited. As this is an Open Source project and I personally don't have a lot of time to investigate everything is details, I need to rely on the knowledge & experience of other developers, especially with really good reputation. @hswong3i had definitely good intentions and correctly understood my goal to make this module as easy to install ass possible, for "normal" end users first of all. This step has been achieved by introducing the CDN support and changing the default path in CKEditor profiles to use it... so I guess we can live without the make file. Also setting the "edit" version as the one suitable for download was the best choice we could make (when deciding to introduce such make file at all).

Anyway, @jcisio - I need you to decide what to do with this. If you have anyone else who can confirm that reverting it the best option, I'm totally okay with a quick release where the make file is gone. I'll definitely not stick and/or defend the decision if it was wrong, just make sure we're not making any mistake again

jcisio’s picture

I was marking the status as "needs review" is waiting for feedback. I even think we are good to revert if there is no feedback in the next few days.

jcisio’s picture

Assigned: hswong3i » Unassigned
Status: Needs review » Fixed

No feedback. I reverted and fixed the CHANGELOG file.

  • Commit 307acb0 on 7.x-1.x by jcisio:
    Issue #2175565 Removed ckeditor.make
    
    This reverts commit...

Status: Fixed » Closed (fixed)

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