Problem/Motivation

Unable to recreate the same source file of ckeditor.js (including images also)

OLD Summary:
------

I ran into a strange problem with Chrome Canary [Version 60.0.3104.0 (Official Build) canary (64-bit) ]. On a fresh installed Drupal site, when I turn off Aggregate JavaScript files and then create a node content. the ckeditor.js has SyntaxError.
(background story of why I found it, not the bug report)

I started to debug and found the ckeditor.js in Drupal repo different to all other files on CKEditor official website.

I also tried to upload the build-config.js from drupal repo to http://ckeditor.com/builder and get different file source.

Proposed resolution

- Update the source files

Remaining tasks

- Standardize the build process in Drupal. Use CKEditor online builder or local build.
- Patch it

Comments

droplet created an issue. See original summary.

Wim Leers’s picture

Category: Bug report » Support request
Priority: Critical » Normal
Status: Active » Postponed (maintainer needs more info)
Issue tags: -ckeditor, -Chrome, -chrome canary, -security +Needs steps to reproduce

No syntax errors for tens of thousands D8 sites. Perhaps a bug in Chrome canary?

Build instructions are in the build config file you cited.

Marking this critical seems rather premature.

droplet’s picture

Category: Support request » Bug report
Issue summary: View changes
Status: Postponed (maintainer needs more info) » Active
Issue tags: -Needs steps to reproduce

No syntax errors for tens of thousands D8 sites. Perhaps a bug in Chrome canary?

Skip it, for now. :P That is another problem, I will file it later if it still exists in next few Chrome Canary build. (Probably I will file an issue at Google / CKEditor more than Drupal)

---

1.
Hmm. I understand we tweaked the build-config.js. But it missing whole "languages" entry, some other entries and different formatting.

2.
If we built online, we able to re-download our files. We should add back the info:

/**
* This file was added automatically by CKEditor builder.
* You may re-use it at any time to build CKEditor again.
*
* If you would like to build CKEditor online again
* (for example to upgrade), visit one the following links:
*
* (1) http://ckeditor.com/builder
* Visit online builder to build CKEditor from scratch.
*
* (2) http://ckeditor.com/builder/9b985694ffe43618d95b62952d894039
* Visit online builder to build CKEditor, starting with the same setup as before.
*
* (3) http://ckeditor.com/builder/download/9b985694ffe43618d95b62952d894039
* Straight download link to the latest version of CKEditor (Optimized) with the same setup as before.
*
* NOTE:
* This file is not used by CKEditor, you may remove it.
* Changing this file will not change your CKEditor configuration.
*/

3.
the local build (via sh build.sh -s, used drupal build-config.js) source files different to Drupal GIT repo. It's not reproducible.

droplet’s picture

plus above, I afraid our minified source is a vulnerability, therefore I marked it Critical. We can't simply figure the problem from minified source.

Wim Leers’s picture

Status: Active » Postponed (maintainer needs more info)

#3:

  1. It's missing the languages key on purpose: that way, CKEditor is built with all languages it has translations for. And the formatting was updated in #2850642: Reformat CKEditor build-config.js to match upstream, so that the diff is minimal.
  2. There have always been problems with their online builder — you must use the build script.
  3. Please do an inline diff: git diff --color-words --word-diff-regex=[^[:space:]]|([[:alnum:]]|UTF_8_GUARD)+. You'll see that the only difference is a randomly generated cache-busting hash. Yes, this is annoying. But it's by design; we can't turn it off. They are fixing this for CKEditor 5, they're aware that this is not ideal.

#4: So because an alpha- or beta-level stability browser is throwing JS errors, and it's hard to debug them because D8 ships with minified source by default, this is a critical security vulnerability? This makes no sense. We must ship with a minified source, otherwise front-end performance suffers massively. When you debug CKEditor, you can replace the minified source with a CKEditor git clone that has the same version checked out.

droplet’s picture

Issue summary: View changes

Please do an inline diff: git diff --color-words --word-diff-regex=[^[:space:]]|([[:alnum:]]|UTF_8_GUARD)+. You'll see that the only difference is a randomly generated cache-busting hash. Yes, this is annoying. But it's by design; we can't turn it off. They are fixing this for CKEditor 5, they're aware that this is not ideal.

I hacked it before comparing the data. So do you able to reproduce the same source?

#4: So because an alpha- or beta-level stability browser is throwing JS errors, and it's hard to debug them because D8 ships with minified source by default, this is a critical security vulnerability? This makes no sense. We must ship with a minified source, otherwise front-end performance suffers massively. When you debug CKEditor, you can replace the minified source with a CKEditor git clone that has the same version checked out.

Nope. I meant if we may ship a vulnerability accidentally. The browser problem just an opening line. (Updated IS)

this is still the development stage of D8.4. I think it's good to re-patch the files again with up-to-date build tool from CKEditor.

droplet’s picture

CKBUILDER_VERSION="2.3.1"
CKBUILDER_URL="http://download.cksource.com/CKBuilder/$CKBUILDER_VERSION/ckbuilder.jar"

CKEDITOR 4.6.2 specify the same version of builder also.
https://github.com/ckeditor/ckeditor-dev/blob/4.6.2/dev/builder/build.sh...

No idea what side-effects affected my build then.